Add openapi built-in toolset type#1719
Conversation
Introduce a new 'openapi' toolset that fetches an OpenAPI JSON spec from a URL and converts each operation into an HTTP tool. This allows agents to interact with any API that publishes an OpenAPI specification without manually defining individual API tools. Implementation: - pkg/tools/builtin/openapi.go: core toolset that parses OpenAPI 3.x specs via kin-openapi, generates tools per operation with proper parameter schemas, and executes HTTP requests with path/query/body parameter handling - pkg/tools/builtin/openapi_test.go: comprehensive tests covering tool generation, parameter schemas, GET/POST/path params, custom headers, error responses, and edge cases - pkg/config/latest: add validation for the openapi type (requires url) and allow the url and headers fields on it - pkg/config/v4: sync Toolset struct with new headers field - pkg/teamloader/registry.go: register the openapi toolset creator and migrate a2a to use the new top-level headers field - cagent-schema.json: add openapi to the type enum, add headers property, and add allOf validation requiring url - examples/openapi-petstore.yaml: example agent using the Swagger Pet Store API - go.mod/go.sum: add github.com/getkin/kin-openapi dependency Assisted-By: cagent
There was a problem hiding this comment.
Code Review Summary
I've reviewed the OpenAPI toolset implementation and found 3 issues that should be addressed:
High-Priority Issues
- Server URL variables not supported (line 150) - OpenAPI specs with server variables like
{basePath}will create broken URLs - HTTP client created per request (lines 71, 381) - This defeats connection pooling and hurts performance
- No SSRF protection on spec URL (line 66) - URL validation needed to prevent internal network access
All findings are on newly added code. The implementation is generally solid with comprehensive tests. Great work on the detailed test coverage!
Note: 2 additional low-severity observations were identified but not included to focus on actionable items.
| } | ||
|
|
||
| // resolveBaseURL determines the base URL for API requests. | ||
| func (t *OpenAPITool) resolveBaseURL(spec *openapi3.T) (string, error) { |
There was a problem hiding this comment.
Server URL variable substitution not implemented
The OpenAPI spec allows server URLs with variables like:
servers:
- url: 'https://api.example.com/{basePath}'
variables:
basePath:
default: v1This code extracts spec.Servers[0].URL directly without substituting variables. This will result in broken URLs like https://api.example.com/{basePath}/pets instead of https://api.example.com/v1/pets.
Suggested fix: Check for variables in the server object and substitute them with their default values before using the URL.
| req.Header.Set("Accept", "application/json") | ||
| setHeaders(req, t.headers) | ||
|
|
||
| resp, err := (&http.Client{Timeout: httpTimeout}).Do(req) |
There was a problem hiding this comment.
HTTP client created per request - defeats connection pooling
Creating a new http.Client for each request is inefficient:
- Defeats HTTP connection pooling (new TCP connections for each call)
- Creates unnecessary goroutines that need garbage collection
- Violates Go HTTP client best practices
The same issue exists at line 381 in callTool().
Suggested fix: Create a single shared http.Client as a package variable or struct field:
var httpClient = &http.Client{Timeout: httpTimeout}Then use httpClient.Do(req) instead of creating new clients.
| req, err := http.NewRequestWithContext(ctx, http.MethodGet, t.specURL, http.NoBody) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("failed to create request: %w", err) | ||
| } |
There was a problem hiding this comment.
No SSRF protection - spec URL should be validated
The specURL from configuration is used directly without validation. If an attacker can control environment variables or configuration, they could point this to:
- Internal IP addresses (127.0.0.1, 10.0.0.0/8, 192.168.0.0/16)
- Localhost services
- Cloud metadata endpoints (169.254.169.254)
While this is config-driven and typically admin-controlled, defense-in-depth suggests validating URLs.
Suggested fix: Add URL validation to reject private IP ranges and localhost addresses, or document this as a security consideration.
| func createOpenAPITool(ctx context.Context, toolset latest.Toolset, _ string, runConfig *config.RuntimeConfig) (tools.ToolSet, error) { | ||
| expander := js.NewJsExpander(runConfig.EnvProvider()) | ||
|
|
||
| specURL := expander.Expand(ctx, toolset.URL) |
There was a problem hiding this comment.
Expanded URL not validated before use
The specURL is expanded from environment variables but never validated. If ${env.MALICIOUS_VAR} contains a private IP or localhost URL, it will be accepted.
This is related to the SSRF concern in openapi.go. Validation should occur here after expansion, before passing to NewOpenAPITool().
Suggested fix:
specURL := expander.Expand(ctx, toolset.URL)
if err := validateURL(specURL); err != nil {
return nil, fmt.Errorf("invalid spec URL: %w", err)
}
Introduce a new 'openapi' toolset that fetches an OpenAPI JSON spec from a URL and converts each operation into an HTTP tool. This allows agents to interact with any API that publishes an OpenAPI specification without manually defining individual API tools.
Implementation:
Assisted-By: cagent