Expose Bazaar schemas from OpenAPI payment challenges - #429
Conversation
|
@epistemedeus is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis change adds Bazaar discovery metadata to payment challenges by projecting the matched OpenAPI operation's parameters, JSON request body, and successful JSON response schema. Focused checks confirmed that literal routes take precedence over matching template routes, concrete requests retain metadata for template routes, operation-level parameter overrides replace inherited requiredness, and optional JSON request bodies remain optional. The pull request does not meet the repository's verified-commit requirement because the current head commit is unsigned. Confidence Score: 4/5Do not merge until the head commit is replaced with a verified signed commit. The remaining blocking issue is the repository rule requiring a verified signature on the current head commit. Files Needing Attention: rust/crates/core/src/lib.rs
 T-Rex LogsWhat T-Rex did
Reviews (2): Last reviewed commit: "Expose Bazaar schemas from OpenAPI chall..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let (path_item, operation) = paths.iter().find_map(|(path, item)| { | ||
| let combined = if base_path.is_empty() { | ||
| normalize_path(path) | ||
| } else { | ||
| format!("{}/{}", base_path, path.trim_start_matches('/')) | ||
| }; | ||
| if canonical_path(&combined) != target { | ||
| return None; |
There was a problem hiding this comment.
When a paid OpenAPI route contains a path parameter, this compares a concrete request path such as projects/123 with the canonicalized template projects/{*} using string equality. The operation is therefore not found and the PAYMENT-REQUIRED challenge omits Bazaar discovery metadata for the route.
Artifacts
Focused Rust regression-test source
- Captures the executed test that calls the Bazaar operation matcher with OpenAPI path `/projects/{id}` and concrete request path `/projects/123`, with the takeaway that concrete paths are required to resolve templated operations.
Observed templated-path test failure
- Shows the focused Rust test ran one test and failed at the assertion that concrete `/projects/123` must resolve Bazaar metadata from `/projects/{id}`, with the takeaway that the finding is reproduced.
| for parameter in path_item | ||
| .get("parameters") | ||
| .and_then(Value::as_array) | ||
| .into_iter() | ||
| .flatten() | ||
| .chain( | ||
| operation | ||
| .get("parameters") | ||
| .and_then(Value::as_array) | ||
| .into_iter() | ||
| .flatten(), | ||
| ) | ||
| { | ||
| let parameter = resolve_openapi_object(doc, parameter)?; | ||
| let location = parameter.get("in").and_then(Value::as_str)?; | ||
| let name = parameter.get("name").and_then(Value::as_str)?.to_string(); | ||
| let normalized_name = name.to_ascii_lowercase(); | ||
| if location == "header" | ||
| && matches!( | ||
| normalized_name.as_str(), | ||
| "authorization" | "payment-signature" | "x-payment" | "payment-required" | ||
| ) | ||
| { | ||
| continue; | ||
| } | ||
| let schema = parameter.get("schema")?; | ||
| let schema = materialize_local_refs(doc, schema, &mut HashSet::new(), 0)?; | ||
| let required = parameter | ||
| .get("required") | ||
| .and_then(Value::as_bool) | ||
| .unwrap_or(location == "path"); | ||
| let example = parameter | ||
| .get("example") | ||
| .or_else(|| parameter.get("schema").and_then(|s| s.get("example"))) | ||
| .or_else(|| parameter.get("schema").and_then(|s| s.get("default"))) | ||
| .cloned(); | ||
| match location { | ||
| "query" => { | ||
| query_properties.insert(name.clone(), schema); | ||
| if let Some(example) = example { | ||
| query_examples.insert(name.clone(), example); | ||
| } | ||
| if required { | ||
| query_required.push(Value::String(name)); | ||
| } | ||
| } | ||
| "path" => { | ||
| path_properties.insert(name.clone(), schema); | ||
| if let Some(example) = example { | ||
| path_examples.insert(name.clone(), example); | ||
| } | ||
| if required { | ||
| path_required.push(Value::String(name)); | ||
| } | ||
| } | ||
| "header" => { | ||
| header_properties.insert(name.clone(), schema); | ||
| if let Some(example) = example { | ||
| header_examples.insert(name.clone(), example); | ||
| } | ||
| if required { | ||
| header_required.push(Value::String(name)); | ||
| } | ||
| } | ||
| _ => {} |
There was a problem hiding this comment.
When an operation overrides a path-level parameter with the same name and location, the property schema is replaced but the old required name remains appended. The generated Bazaar contract can therefore combine the operation-level schema with path-level requiredness and reject a valid invocation.
| if body_method { | ||
| let request_body = operation.get("requestBody")?; | ||
| let request_body = resolve_openapi_object(doc, request_body)?; | ||
| let media = json_media_type(request_body.get("content")?.as_object()?)?; | ||
| let body_schema = | ||
| materialize_local_refs(doc, media.get("schema")?, &mut HashSet::new(), 0)?; | ||
| input_info.insert("bodyType".to_string(), json!("json")); | ||
| input_info.insert( | ||
| "body".to_string(), | ||
| media.get("example").cloned().unwrap_or_else(|| json!({})), | ||
| ); | ||
| input_schema_properties.insert( | ||
| "bodyType".to_string(), | ||
| json!({ "type": "string", "enum": ["json"] }), | ||
| ); | ||
| input_schema_properties.insert("body".to_string(), body_schema); | ||
| input_required.extend([json!("bodyType"), json!("body")]); |
There was a problem hiding this comment.
For POST, PUT, and PATCH operations with an OpenAPI JSON requestBody marked required: false, this always adds bodyType and body to the required input fields. Bazaar clients are consequently told that bodyless invocations are invalid even though the operation accepts them.
Artifacts
Optional request body reproduction source
- The executable Rust test constructs an OpenAPI POST requestBody with required:false and asserts that Bazaar does not require body fields; it is the focused reproduction source.
Optional request body reproduction output
- Cargo executed the focused test and printed the Bazaar input schema requiring bodyType and body before the expected assertion failure; the finding is reproduced.
| /// Filtered OpenAPI document served by this gateway, when available. | ||
| /// The payment gate uses it only to derive optional machine-discovery | ||
| /// metadata for the exact method and route being challenged. | ||
| fn openapi_document(&self) -> Option<&serde_json::Value> { | ||
| None | ||
| } |
There was a problem hiding this comment.
Commit 098782b9155c7400711e8f9c5f5ae1d3288b311b has no verified cryptographic signature, so this pull request does not satisfy the repository owner’s signed-commit merge requirement.
Context Used: Request changes if the commits are not signed (ver... (source)
098782b to
d50886b
Compare
|
Updated the branch with GitHub-verified signed commit |
| #[cfg(feature = "server")] | ||
| pub trait PaymentState: Clone + Send + Sync + 'static { | ||
| fn apis(&self) -> &[ApiSpec]; | ||
| /// Filtered OpenAPI document served by this gateway, when available. |
There was a problem hiding this comment.
The current PR head commit d50886b965c32caf77e1817cde16b8609c42cab4 has no cryptographic signature, so the pull request still violates the repository owner's verified-commit merge requirement.
Context Used: Request changes if the commits are not signed (ver... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
1 participant
Summary
Verification
cargo test -p pay-core --features server bazaar_projection --libcargo test -p pay-core --features server x402_extension_attachment_preserves_existing_keys --libcargo check -p paycargo clippy -p pay-core --features server --lib -- -D warningsFixes #428