feat(evidence): add progressive client API and tutorial - #735
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6c15eebc0e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if args.client.is_some() || args.subjects_file.is_some() || args.purpose.is_some() { | ||
| bail!("progressive preparation accepts profile, requirement, and direct subject fields"); |
There was a problem hiding this comment.
When --profile selects progressive preparation, this rejects --subjects-file, while the direct parser also rejects the documented role:field=value syntax and constructs only a flat AudienceScopedRequest. Any definition with multiple request-origin roles—such as the coequal legal-parent relationship acceptance definition—therefore fails definition selection before a request is prepared, even though the generated CLI reference advertises both multi-subject input forms.
AGENTS.md reference: AGENTS.md:L139-L142
Useful? React with 👍 / 👎.
| && !value.chars().any(char::is_control) | ||
| }) | ||
| .ok_or_else(|| anyhow!("progressive subject must be one bounded field=value"))?; | ||
| Ok((field.to_owned(), SelectorValue::String(value.to_owned()))) |
There was a problem hiding this comment.
When a published selector field is integer or boolean, every --subject field=value is unconditionally converted to SelectorValue::String. Progressive definition matching requires the exact published scalar type, so valid requests for those contract-supported selector forms always fail locally with the generic preparation error and the CLI provides no typed alternative because --subjects-file is rejected in this mode.
Useful? React with 👍 / 👎.
| match url.scheme() { | ||
| "https" if !allow_local_loopback => Ok(()), | ||
| "http" if allow_local_loopback && url.host().is_some_and(is_loopback_host) => Ok(()), | ||
| "https" if allow_local_loopback => { |
There was a problem hiding this comment.
When --local-loopback-discovery is used with --base-url http://127.0.0.1 and no explicit port, this validation accepts the URL and reports that the profile was created, but EvidenceClientProfile::validate later requires base_url.port() to be present. The generated profile is therefore unusable by contracts fetch, request prepare, and both SDK bindings; reject the input during creation or generate a URL satisfying the core profile contract.
Useful? React with 👍 / 👎.
| fn create_profile(args: ProfileCreateArgs) -> Result<ExitCode> { | ||
| validate_new_output(&args.out).context("client profile output is unsafe")?; | ||
| validate_base_url(&args.base_url, args.local_loopback_discovery)?; | ||
| validate_nonempty_identifier(&args.client_id, "client identifier")?; |
There was a problem hiding this comment.
When --client-id is between 257 and 2,048 bytes, this check accepts it and profile create writes the owner-only output successfully, but the SDK's EvidenceClientProfile::validate rejects every client ID longer than 256 bytes. The newly created profile consequently cannot be used by contract discovery, request preparation, Python, or Node, so creation should apply the same 256-byte bound rather than the generic identifier limit.
Useful? React with 👍 / 👎.
| if let Some(cached) = cache | ||
| .as_ref() | ||
| .filter(|cached| cached.stale_until > Instant::now()) | ||
| { | ||
| return Ok(cached.value.clone()); |
There was a problem hiding this comment.
When any protected-resource, authorization-server, or JWKS revalidation fails after the advertised cache lifetime, this fallback continues using the expired service snapshot; stale_until extends that use by another 900 seconds beyond maximumMetadataCacheSeconds. During a signing-key rotation, a client that already waited the documented maximum 600-second propagation interval can therefore retain the predecessor key set and reject assertions from the newly active key—or continue trusting a key the refreshed JWKS removed—for up to 15 additional minutes.
Useful? React with 👍 / 👎.
| python3 -m venv .venv | ||
| . .venv/bin/activate | ||
|
|
||
| VERSION=0.20.0 |
There was a problem hiding this comment.
The tutorial downloads v0.20.0 bindings and then calls EvidenceClient.from_profile/fromProfile, but this API is introduced by this commit on a workspace already versioned 0.20.1; the release inventory derives client asset versions from that workspace version. A v0.20.0 wheel or tarball therefore cannot contain the demonstrated API, so both SDK lanes fail before making a request even if those older assets exist; use the release version that actually includes this change.
AGENTS.md reference: docs/site/AGENTS.md:L25-L27
Useful? React with 👍 / 👎.
| pub struct ExpectedListDocument { | ||
| pub items: ExpectedListItemFormDocument, | ||
| #[serde(deserialize_with = "read_minimum_items")] | ||
| pub minimum_items: usize, | ||
| #[serde(deserialize_with = "read_maximum_items")] | ||
| pub maximum_items: usize, | ||
| pub unique: bool, |
There was a problem hiding this comment.
When re-verifying a retained pre-change response whose policy expects a list output, deserialization now requires the newly added items and unique members and fails before signature or policy verification. The custom compatibility deserializer supplies defaults for the simultaneously added output handle and required fields, but no equivalent legacy representation exists for the old {minimumItems, maximumItems} list shape; consequently archived Version 1 audience-scoped and holder-bound list assertions become unverifiable after upgrading.
Useful? React with 👍 / 👎.
| static fromProfile(path, privateKeyJwk) { | ||
| try { | ||
| return privateKeyJwk === undefined | ||
| ? native.EvidenceClient.fromProfile(path) | ||
| : native.EvidenceClient.fromProfile(path, privateKeyJwk); |
There was a problem hiding this comment.
When callers construct the progressive client through the exported EvidenceClient.fromProfile, this explicitly invokes the native base-class factory and returns a native.EvidenceClient, not an instance of the exported JavaScript subclass. The methods happen to work because the native prototype was patched, but client instanceof EvidenceClient is false and any consumer subclassing or prototype extension is bypassed, contradicting the declared EvidenceClient return type.
Useful? React with 👍 / 👎.
| let bytes = fs::read(self.resolve(file)).map_err(|_| profile_error())?; | ||
| if bytes.len() > 1024 * 1024 { | ||
| return Err(profile_error()); |
There was a problem hiding this comment.
When a profile references an accidentally or maliciously oversized JWKS or reviewed-contract file, fs::read allocates and reads the complete file before the nominal 1 MiB or 4 MiB limit is checked, so those limits do not prevent memory exhaustion. from_file has the same issue for its 256 KiB profile bound, and file-backed private keys are read without any bound at all; use metadata plus a bounded reader so startup cannot allocate attacker-sized local artifacts before refusing them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b75e6cc928
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let origin_only = base_url.path() == "/" | ||
| && !self.base_url.ends_with('/') |
There was a problem hiding this comment.
When an application-authored profile uses a parseable but noncanonical origin such as https://EXAMPLE.com or an explicit default port, this check accepts it because it validates only the parsed URL shape. Metadata acquisition later compares ProtectedResourceMetadata.resource to the original profile.base_url string exactly (client.rs:518), while the Evidence runtime publishes only canonical origins, so every metadata refresh, contract fetch, and request fails despite the profile passing validate(). Require the stored string to equal the parsed origin's canonical serialization here.
Useful? React with 👍 / 👎.
| prototype.request = function (...args) { | ||
| return original.apply(this, args).then( | ||
| (result) => result, |
There was a problem hiding this comment.
When request() receives an invalid shape, such as missing both selectors and subjects or carrying an unsupported response format, the native Rust method returns Err before spawn_future, so original.apply(...) throws synchronously. Because that invocation is outside a try block, this wrapper never calls normalize and the caller receives the raw JSON-message Error instead of the documented EvidenceClientError; catch synchronous throws around the invocation as well as Promise rejections.
Useful? React with 👍 / 👎.
| || response | ||
| .headers() | ||
| .get(CONTENT_TYPE) | ||
| .and_then(|value| value.to_str().ok()) | ||
| .is_none_or(|value| value.split(';').next() != Some(media_type)) |
There was a problem hiding this comment.
When an external authorization server returns a valid JSON media type with different casing, such as Application/JSON, this exact string comparison rejects its metadata before token acquisition even though HTTP media types are case-insensitive. The protected-resource and JWKS fetches share this helper, and the client's ordinary response path already uses a case-insensitive essence comparison; apply the same parsing here so conforming authorization-server metadata is interoperable.
Useful? React with 👍 / 👎.
| evidencectl access policy add first-assertion-policy --question adult-status | ||
| evidencectl access client add first-assertion-client \ | ||
| --policy first-assertion-policy \ | ||
| --generate-local-key |
There was a problem hiding this comment.
Adding this first access policy permanently switches the shared adult-status project from its implicit tutorial caller to registered-client mode; dev clean removes only the generated dev session, not access/policies or .evidence/clients. The replay script then reuses or copies this project for request-evidence-as-sd-jwt-vc, return-a-governed-value, and refuse-unsafe-evidence-requests, whose documented request prepare commands provide no --client, so they now stop with “the active project requires a registered client” instead of completing. Keep the shared starter unauthenticated or update every dependent journey to create and select a client.
AGENTS.md reference: docs/site/AGENTS.md:L25-L27
Useful? React with 👍 / 👎.
b75e6cc to
7b24b07
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b24b07c7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Linux-x86_64) PLATFORM="linux_x86_64" ;; | ||
| Linux-aarch64|Linux-arm64) PLATFORM="linux_aarch64" ;; |
There was a problem hiding this comment.
On both supported Linux branches, this constructs the obsolete linux_x86_64/linux_aarch64 wheel names, so curl -fLO returns 404 and the Python tutorial lane stops before installation. The fresh evidence after the version update is the v0.21.1 release inventory in release/scripts/release_candidate.py:141-150, which publishes manylinux_2_17_x86_64.manylinux2014_x86_64 and manylinux_2_17_aarch64.manylinux2014_aarch64 wheel suffixes instead.
AGENTS.md reference: docs/site/AGENTS.md:L25-L27
Useful? React with 👍 / 👎.
| for concept in &definition.concepts { | ||
| let expected = if concept.form.is_list() { | ||
| concept.list_expected_output(1, MAXIMUM_LIST_ITEMS) | ||
| concept.list_expected_output() |
There was a problem hiding this comment.
When a valid holder-bound definition permits sd-jwt-vc but not sd-jwt-vc-batch, this function still publishes it as an OID4VCI credential configuration. Every resulting request then hardcodes SdJwtVcBatch in offer.rs:129-130 and is sent through send_holder_bound_batch in issuer.rs:150-156, so Evidence refuses issuance for a credential the wallet was explicitly offered; filter these catalog entries by the definition's effective response formats, or use the single-credential path when only that format is available.
Useful? React with 👍 / 👎.
| .into_iter() | ||
| .flatten() | ||
| { | ||
| validate_bounded_identifier(value, 2_048, "expected identity")?; |
There was a problem hiding this comment.
When profile create receives an expected audience, issuer, or provider that is not a URI—or is between 513 and 2,048 bytes—this generic identifier check accepts it and writes a supposedly valid profile. The published client-profile contract restricts each field to a URI of at most 512 bytes, and progressive contract validation applies the same URI bound before comparing identities, so no discovered service can satisfy the generated profile; contract discovery and OAuth can occur before every request fails with an identity mismatch.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea8bf7fed0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .public_json( | ||
| metadata_url, | ||
| JSON_MEDIA_TYPE, | ||
| previous.map(|value| { |
There was a problem hiding this comment.
When protected-resource revalidation announces a different authorization server, this unconditionally sends the previous server's ETag to the new metadata URL and may reuse the previous document on a 304. If both servers use a common tag such as "1", the reused document fails the new issuer check; because the old snapshot is then restored, every subsequent refresh repeats the same conditional request and the client remains unusable until restart. Reuse the authorization metadata and ETag only when the announced issuer/metadata URL is unchanged.
Useful? React with 👍 / 👎.
| --policy first-assertion-policy \ | ||
| --generate-local-key | ||
| evidencectl client profile create \ | ||
| --base-url http://127.0.0.1:8080 \ |
There was a problem hiding this comment.
When a reader follows the later “If local ports are already in use” instructions and starts Evidence on port 8180, this newly added profile remains pinned to port 8080. The progressive CLI and both SDK paths resolve discovery and request endpoints from this exact application-owned baseUrl, so they contact the unused port despite the tutorial's claim that the selected ports are used consistently. Tell readers to create the profile with the alternate Evidence port as well.
AGENTS.md reference: docs/site/AGENTS.md:L25-L27
Useful? React with 👍 / 👎.
| let state = self.progressive_state()?; | ||
| let snapshot = self.progressive_snapshot(state).await?; | ||
| Ok(snapshot.definitions.into()) |
There was a problem hiding this comment.
When a Rust caller invokes contracts_candidate() on a profile whose contracts mode is reviewed, progressive_snapshot loads the local reviewed file and this method returns it unchanged instead of fetching the requester-scoped catalog from the deployment. That makes the advertised review workflow unable to detect contract changes without reconstructing or mutating the profile; evidencectl client contracts fetch already has to work around this by forcing ContractsProfile::Published before calling the same method. Make this candidate-fetching API bypass the reviewed source as the CLI does.
Useful? React with 👍 / 👎.
| ArgGroup::new("subject_input") | ||
| .required(true) | ||
| .args(["subject", "subjects_file"]) | ||
| ), group( |
There was a problem hiding this comment.
When a published definition derives every subject selector from authenticated-context or authenticated-grant, the progressive core correctly selects it only when the caller supplies an empty selector map, but this unconditional Clap group requires either --subject or --subjects-file. Any supplied entry then makes definition_shape_matches reject the definition, so evidencectl request prepare --profile cannot prepare valid fully authentication-derived definitions such as the selector conformance cases. Make subject input optional in progressive mode and let the selected contract decide whether request-origin values are required.
Useful? React with 👍 / 👎.
| if self.schema != EVIDENCE_CLIENT_PROFILE_SCHEMA_V1 | ||
| || self.client_id.is_empty() | ||
| || self.client_id.len() > 256 |
There was a problem hiding this comment.
When an application parses a profile containing an empty or oversized private-key path, environment variable name, pinned-JWKS path, or reviewed-contract path, validate() accepts it even though the published profile schema requires non-empty bounded paths and a syntactically valid environment variable. from_slice() therefore reports such a profile as valid, while later construction or the first metadata-backed request fails when the reference is resolved; reviewed profiles can even perform metadata discovery before discovering that their local contract reference is unusable. Apply the schema's reference checks during profile validation.
AGENTS.md reference: AGENTS.md:L144-L145
Useful? React with 👍 / 👎.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
ea8bf7f to
c0a7a18
Compare
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9dcd10d536
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let snapshot = self.progressive_snapshot(state).await?; | ||
| let definition = select_definition(&snapshot.definitions, &request)?.clone(); |
There was a problem hiding this comment.
When a Rust caller sets EvidenceResponseFormat::SdJwtVcBatch (which the Python binding also accepts during enum deserialization), this starts progressive_snapshot before select_definition rejects the format. With published contracts, snapshot acquisition performs discovery and obtains/sends an OAuth bearer token to fetch the definitions catalog, so an input that is always invalid for this API causes authenticated network activity before returning a local configuration error. Validate the format before acquiring the snapshot in both request and prepare_progressive.
Useful? React with 👍 / 👎.
| refuse_changed_file(&parent, &leaf, expected)?; | ||
| rustix::fs::renameat_with(&parent, name, &parent, &leaf, RenameFlags::empty()) | ||
| .context("publishing the updated mock config")?; |
There was a problem hiding this comment.
When an editor or another source mock generate process replaces source.yaml after the second refuse_changed_file call returns but before this rename executes, the unconditional rename overwrites those fresh edits. The pre-rename byte comparison therefore does not provide the advertised compare-and-replace behavior, and the append operation can silently lose a concurrent authoring change; use an operation that verifies the displaced inode/content as part of the replacement protocol or otherwise serialize writers.
Useful? React with 👍 / 👎.
| let url = Url::parse(value) | ||
| .map_err(|_| ConfigError::Invalid("service publicOrigin is not a canonical origin"))?; |
There was a problem hiding this comment.
When a bundle supplies a canonical HTTPS origin longer than 512 characters, this validator accepts it even though products/evidence/contracts/bundle.schema.yaml caps service.publicOrigin at 512. Because the runtime then embeds the unchecked value in protected-resource metadata and in the WWW-Authenticate header attached to every 401, a directly loaded configuration can produce headers approaching the 1 MiB bundle limit and fail clients despite being reported as valid; apply the same URI length bound used for the other service identities.
Useful? React with 👍 / 👎.
1 participant
What changed
Why
The existing advanced client was safe but too verbose for a first integration. Applications had to assemble discovery-derived procedure fields, token configuration, and local verification steps manually. This adds a progressive surface:
The lower-level prepare, send, verify, batch, and holder-bound APIs remain available.
The client does not persist application subject relationships. First-use verification returns an opaque, scoped receipt that the application may atomically retain with its own account or case record.
Security and trust boundaries
Validation
cargo fmt --checkcargo check --locked --workspace --all-targetsnpm testindocs/site(332 tests)npm run checkindocs/sitefirst-evidence-assertion, including signed JWS and audience-scoped SD-JWT VC pathsgit diff --checkReview note
The separate
return-a-governed-valuefollow-up tutorial currently stops during Evidence project compilation because its requirement role and selector profile lack an authority path. The first-assertion tutorial and its generated records pass; that follow-up failure occurs before the generated child record is read and is outside this PR's first-tutorial scope.