fix(openapi): skip auth injection for public operations

[jolestar]

What

• add operation-level auth requirement evaluation in OpenAPI adapter using security metadata
• skip all auth injection (URL + headers) when an operation is explicitly public
• keep backward compatibility when auth metadata is unknown by preserving current injection behavior
• annotate Binance Spot signed endpoints with security and add components.securitySchemes.api_key
• add tests covering Public / RequiresAuth / Unknown and execute-path auth gating

Why

Issue #189 reports signer/query injection being applied to public endpoints such as GET /api/v3/ping, causing Binance to reject requests with -1101 Too many parameters.

How

• Introduced OperationAuthRequirement (Public, RequiresAuth, Unknown) in src/adapters/openapi.rs
• Evaluated security from operation-level security, then root-level security, with OpenAPI semantics handling for mixed secured/public operations
• Gated both apply_auth_profile_to_url and apply_auth_profile in execute() based on requirement
• Updated skills/binance-spot-openapi-skill/references/binance-spot.openapi.json to explicitly mark signed operations and declare api_key security scheme

Testing

• cargo fmt -- --check
• cargo test --package uxc openapi::tests:: -- --nocapture

Closes #189

[Copilot]

Pull request overview

This PR updates the OpenAPI adapter to evaluate OpenAPI/Swagger security metadata per operation so that auth (URL/query signing + header injection) is skipped for operations that are explicitly public, addressing Binance public endpoints being rejected when signed.

Changes:

• Added OperationAuthRequirement evaluation from operation-level and root-level security, with a fallback heuristic for mixed public/secured schemas.
• Gated apply_auth_profile_to_url and apply_auth_profile in execute() so public operations do not receive auth injection.
• Annotated Binance Spot signed endpoints with security and added components.securitySchemes.api_key to the reference OpenAPI document.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/adapters/openapi.rs	Implements per-operation auth requirement evaluation and gates auth injection; adds unit/integration tests for Public/RequiresAuth/Unknown behaviors.
skills/binance-spot-openapi-skill/references/binance-spot.openapi.json	Adds api_key security scheme and marks signed endpoints with operation-level security to enable correct auth gating.

Comments suppressed due to low confidence (1)

src/adapters/openapi.rs:1086

• execute() always uses send_with_oauth_retry(), which refreshes OAuth profiles (and can trigger network refreshes) even when auth_requirement is Public and auth injection is skipped. To fully skip auth work for public operations, consider bypassing OAuth refresh/retry in this branch (e.g., build the request with None profile and send directly).

        let auth_requirement = Self::operation_auth_requirement(operation_spec, &schema);
        let should_apply_auth = !matches!(auth_requirement, OperationAuthRequirement::Public);

        let resp = self
            .send_with_oauth_retry(|profile| {
                let full_url = {

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

schema_has_any_operation_security() walks the entire paths map to detect any security usage. This is called for every execution when both operation/root security are absent, so mixed schemas can end up doing an O(schema_size) scan per request. Consider computing/caching this once per fetched schema (e.g., memoize a boolean alongside the cached schema) and reusing it in operation_auth_requirement().

[holonbot]

Review for PR #191: fix(openapi): skip auth injection for public operations

Summary

This PR addresses issue #189 by implementing operation-level auth requirement evaluation using OpenAPI/Swagger security metadata. The implementation correctly skips auth injection (both URL query params and headers) for explicitly public operations while maintaining backward compatibility for schemas without security metadata.

Overall Assessment: LGTM with minor suggestions

The implementation is sound, well-tested, and follows OpenAPI semantics correctly. All tests pass, code is properly formatted, and no clippy warnings were raised.

Key Findings

Strengths

1. Correct OpenAPI Semantics (src/adapters/openapi.rs:810-870)

   • Properly implements the three-tier security evaluation: operation-level security → root-level security → implicit public when other operations have security
   • The security_requirement function correctly handles empty arrays {} as public and non-empty objects as requiring auth
   • schema_has_any_operation_security properly implements the OpenAPI 3.x behavior where operations without security are public when ANY operation in the schema defines security

2. Backward Compatibility (src/adapters/openapi.rs:1081-1082)

   • The Unknown case ensures existing behavior is preserved for schemas without security metadata
   • should_apply_auth defaults to true for unknown cases, preventing breaking changes

3. Comprehensive Test Coverage (src/adapters/openapi.rs:1293-1463)

   • Unit tests cover all three cases (Public, RequiresAuth, Unknown)
   • Integration tests verify both URL and header auth injection are correctly skipped/applied
   • Tests use mock servers with proper query parameter matching

4. Binance Schema Annotations (skills/binance-spot-openapi-skill/references/binance-spot.openapi.json)

   • Correctly adds security: [{"api_key": []}] to 9 signed endpoints
   • Properly defines components.securitySchemes.api_key with correct X-MBX-APIKEY header name
   • Public endpoints like /api/v3/ping remain without security annotations

Minor Suggestions

1. Documentation Clarity (src/adapters/openapi.rs:834-847)
   The schema_has_any_operation_security function could benefit from a doc comment explaining the OpenAPI 3.x specification behavior it implements. This would help future maintainers understand why operations without explicit security are treated as public in this context.

2. Enum Visibility (src/adapters/openapi.rs:882-887)
   Consider whether OperationAuthRequirement should be pub(crate) or pub if it might be useful for testing or debugging in other modules. Currently it's private, which is fine, but making it more visible could aid in diagnostics.

Security Analysis

No security concerns identified.

The implementation correctly handles security requirements:

• Public operations explicitly skip both URL query param injection (apply_auth_profile_to_url) and header injection (apply_auth_profile)
• The three-state design (Public/RequiresAuth/Unknown) prevents accidental auth exposure
• Backward compatibility via Unknown ensures existing behavior isn't silently changed

Performance Analysis

No performance concerns.

• operation_auth_requirement is called once per request and performs straightforward JSON traversal
• schema_has_any_operation_security could be optimized by caching the result per schema, but this is unnecessary given schemas are already cached
• The additional overhead is negligible compared to network I/O

Testing Verification

• All 20 OpenAPI adapter tests pass
• Code passes cargo fmt -- --check
• No clippy warnings in modified files
• Tests verify the exact issue from #189 is fixed (public endpoints don't receive auth params)

Recommendation

APPROVE - This PR correctly implements the fix for issue #189 with proper test coverage, backward compatibility, and adherence to OpenAPI specification semantics. The minor suggestions above are optional improvements that don't block merge.