Allow insecure HTTPS connections (self-signed certificates) #46
Conversation
WalkthroughAdds an optional insecure HTTPS mode across CLI, types, code generation, and security utilities. The CLI exposes -k/--insecure, plumbs the flag through generator code to security utilities, and when enabled, generated Axios requests use an https.Agent with rejectUnauthorized: false. README documents the new option. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (openapi-mcp)
participant Gen as Server Code Generator
participant Sec as Security Utils
participant Axios as Axios
participant HTTPS as https.Agent
User->>CLI: Run with -k/--insecure
CLI->>Gen: options { insecure: true, ... }
Gen->>Sec: generateExecuteApiToolFunction(securitySchemes, insecure=true)
Note over Sec: Build request code<br/>with optional httpsAgent
rect rgba(220,240,255,0.5)
Sec->>Axios: Acquire OAuth2 token<br/>(include httpsAgent if insecure)
Axios->>HTTPS: Agent(rejectUnauthorized=false)
Axios-->>Sec: Token response
end
Sec->>Axios: Execute API request<br/>(include httpsAgent if insecure)
Axios->>HTTPS: Agent(rejectUnauthorized=false)
Axios-->>Gen: API response
Gen-->>CLI: Generated code uses insecure agent when flagged
CLI-->>User: Generation complete
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/utils/security.ts (2)
112-115: Bug: OAuth2 env var names are hard-coded at codegen time (always SCHEMENAME).Inside the generated function, env vars must be derived from the runtime
schemeName, not from a literal'schemeName'. As written, token acquisition will never find credentials.Apply:
- const clientId = process.env[`OAUTH_CLIENT_ID_SCHEMENAME`]; - const clientSecret = process.env[`OAUTH_CLIENT_SECRET_SCHEMENAME`]; - const scopes = process.env[`OAUTH_SCOPES_SCHEMENAME`]; + const sanitized = schemeName.replace(/[^a-zA-Z0-9]/g, '_').toUpperCase(); + const clientId = process.env[`OAUTH_CLIENT_ID_${sanitized}`]; + const clientSecret = process.env[`OAUTH_CLIENT_SECRET_${sanitized}`]; + const scopes = process.env[`OAUTH_SCOPES_${sanitized}`];
136-157: Password flow declared but not implemented (wrong grant_type, missing credentials).You select password flow but still use
grant_type=client_credentialsand never send username/password. Either fully support password flow or drop it.Minimal fix (support both flows):
- let tokenUrl = ''; - if (scheme.flows?.clientCredentials?.tokenUrl) { - tokenUrl = scheme.flows.clientCredentials.tokenUrl; - console.error(`Using client credentials flow for '${schemeName}'`); - } else if (scheme.flows?.password?.tokenUrl) { - tokenUrl = scheme.flows.password.tokenUrl; - console.error(`Using password flow for '${schemeName}'`); - } else { + let tokenUrl = ''; + const useClientCreds = !!scheme.flows?.clientCredentials?.tokenUrl; + const usePassword = !!scheme.flows?.password?.tokenUrl; + if (useClientCreds) { + tokenUrl = scheme.flows.clientCredentials.tokenUrl; + console.error(`Using client credentials flow for '${schemeName}'`); + } else if (usePassword) { + tokenUrl = scheme.flows.password.tokenUrl; + console.error(`Using password flow for '${schemeName}'`); + } else { console.error(`No supported OAuth2 flow found for '${schemeName}'`); return null; } - let formData = new URLSearchParams(); - formData.append('grant_type', 'client_credentials'); + let formData = new URLSearchParams(); + formData.append('grant_type', usePassword ? 'password' : 'client_credentials'); + if (usePassword) { + const roUser = process.env[`OAUTH_USERNAME_${sanitized}`]; + const roPass = process.env[`OAUTH_PASSWORD_${sanitized}`]; + if (!roUser || !roPass) { + console.error(`Missing OAUTH_USERNAME_/OAUTH_PASSWORD_ for password flow '${schemeName}'`); + return null; + } + formData.append('username', roUser); + formData.append('password', roPass); + }Also add these types at the top (Line 16):
| 'OAUTH_CLIENT_SECRET' | 'OAUTH_TOKEN' | 'OAUTH_SCOPES' + | 'OAUTH_USERNAME' + | 'OAUTH_PASSWORD'And consider documenting the new env vars in README.
🧹 Nitpick comments (8)
src/utils/security.ts (3)
160-170: Add timeouts to outbound token requests.Avoid hanging the server on network stalls.
const response = await axios({ method: 'POST', url: tokenUrl, + timeout: 30000, headers: { 'Content-Type': 'application/x-www-form-urlencoded', 'Authorization': `Basic ${Buffer.from(`${clientId}:${clientSecret}`).toString('base64')}` }, data: formData.toString(), ${insecure ? 'httpsAgent: new https.Agent({ rejectUnauthorized: false })' : ''} });
441-448: Also add request timeout to regular API calls.Same stall risk on tool execution requests.
const config: AxiosRequestConfig = { method: definition.method.toUpperCase(), url: requestUrl, params: queryParams, headers: headers, + timeout: 30000, ...(requestBodyData !== undefined && { data: requestBodyData }), ${insecure ? 'httpsAgent: new https.Agent({ rejectUnauthorized: false })' : ''} };
236-263: Security logging: reduce verbosity of “missing credentials” messages.These
console.errorlines run on every tool selection attempt and can spam logs. Considerconsole.debugor gating behind an env flag.README.md (1)
62-63: Add an explicit warning about insecure mode and its scope.Clarify that:
- TLS verification is disabled for BOTH OAuth2 token acquisition and API requests.
- Use only in development/testing; not recommended for production.
Proposed addition below the option:
⚠️ Warning: --insecure disables TLS certificate verification for OAuth2 token acquisition and generated API requests. Use only for development against trusted endpoints (e.g., self‑signed certs). Do not use in production.src/index.ts (1)
90-94: Define--insecureas a true boolean flag with defaultfalse.Using a parser for a flag without a value is brittle. Commander handles booleans natively; presence => true. Set an explicit default.
- .option( - '-k, --insecure', - 'Allow insecure HTTPS connections (self-signed certificates)', - (val) => normalizeBoolean(val) - ) + .option( + '-k, --insecure', + 'Allow insecure HTTPS connections (self-signed certificates)', + false + )If you prefer accepting explicit values (e.g.,
--insecure=false), switch to an optional value and parser:- .option('-k, --insecure', 'Allow insecure HTTPS connections (self-signed certificates)', false) + .option('-k, --insecure [boolean]', 'Allow insecure HTTPS connections (self-signed certificates)', (val) => normalizeBoolean(val) ?? true, false)src/generator/server-code.ts (3)
108-109: Only importhttpswhen--insecureis enabled (avoid unused import in generated server).Currently always importing
httpscauses an unused import when not in insecure mode.- import axios, { type AxiosRequestConfig, type AxiosError } from 'axios'; - import https from 'https'; + import axios, { type AxiosRequestConfig, type AxiosError } from 'axios'; + ${options.insecure ? "import https from 'https';" : ''}
191-218: Consider surfacingerror.causeandresponse.headersin formatted errors.Improves debuggability without leaking secrets.
441-448: Mirror timeout here too (consistency with security.ts token request).Add
timeout: 30000to axios config in the generated code (see earlier comment).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(1 hunks)src/generator/server-code.ts(2 hunks)src/index.ts(1 hunks)src/types/index.ts(1 hunks)src/utils/security.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/index.ts (1)
src/utils/helpers.ts (1)
normalizeBoolean(118-127)
🔇 Additional comments (3)
src/utils/security.ts (1)
205-209: OK: insecure flag correctly plumbed to OAuth2 codegen.Flag flows through to token acquisition generation.
src/types/index.ts (1)
38-40: LGTM: New CLI option is typed and documented.
insecure?: booleanadded toCliOptionswith a clear docstring.src/generator/server-code.ts (1)
37-39: Flag correctly forwarded to security codegen.
options.insecureis passed through; good.
1 participant
Add a command line option to allow insecure HTTPS connections.
Summary by CodeRabbit
New Features
Documentation