feat(client): add HTTPS agent configuration for secure requests#3
feat(client): add HTTPS agent configuration for secure requests#3daniel89fg wants to merge 1 commit intocristotodev:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the FacturaScripts HTTP client to support HTTPS scenarios where local/self-signed certificates are used (e.g., localhost with SSL), by conditionally configuring an Axios httpsAgent.
Changes:
- Import Node’s
httpsmodule and create anhttps.AgentwhenNODE_TLS_REJECT_UNAUTHORIZED === '0'. - Pass the constructed
httpsAgentinto the Axios instance configuration.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const httpsAgent = process.env.NODE_TLS_REJECT_UNAUTHORIZED === '0' | ||
| ? new https.Agent({ rejectUnauthorized: false }) | ||
| : undefined; | ||
|
|
||
| this.client = axios.create({ | ||
| baseURL: `${env.FS_BASE_URL}/api/${env.FS_API_VERSION}`, |
There was a problem hiding this comment.
Using NODE_TLS_REJECT_UNAUTHORIZED === '0' to disable certificate verification makes it easy to accidentally run with TLS validation turned off (MITM risk). Consider using an explicit, library-scoped env var (validated in env.ts) and/or restricting insecure mode to localhost/non-production, and emitting a clear warning when it’s enabled.
| const httpsAgent = process.env.NODE_TLS_REJECT_UNAUTHORIZED === '0' | |
| ? new https.Agent({ rejectUnauthorized: false }) | |
| : undefined; | |
| this.client = axios.create({ | |
| baseURL: `${env.FS_BASE_URL}/api/${env.FS_API_VERSION}`, | |
| const baseURL = `${env.FS_BASE_URL}/api/${env.FS_API_VERSION}`; | |
| const insecureEnv = process.env.FS_CLIENT_INSECURE_TLS; | |
| const insecureRequested = | |
| insecureEnv === '1' || | |
| insecureEnv?.toLowerCase() === 'true'; | |
| let httpsAgent: https.Agent | undefined; | |
| if (insecureRequested) { | |
| let targetUrl: URL | undefined; | |
| try { | |
| targetUrl = new URL(baseURL); | |
| } catch { | |
| // If baseURL cannot be parsed, fall back to secure behavior. | |
| targetUrl = undefined; | |
| } | |
| const hostname = targetUrl?.hostname; | |
| const isLocalhost = | |
| hostname === 'localhost' || | |
| hostname === '127.0.0.1' || | |
| hostname === '::1'; | |
| const isNonProduction = process.env.NODE_ENV !== 'production'; | |
| if (isLocalhost || isNonProduction) { | |
| console.warn( | |
| '[FacturaScriptsClient] Insecure TLS mode enabled: certificate verification is disabled for requests to', | |
| baseURL, | |
| '- do not use this mode in production.' | |
| ); | |
| httpsAgent = new https.Agent({ rejectUnauthorized: false }); | |
| } else { | |
| console.warn( | |
| '[FacturaScriptsClient] FS_CLIENT_INSECURE_TLS is set, but insecure TLS is not allowed for non-localhost targets in production. Using secure TLS instead.' | |
| ); | |
| } | |
| } | |
| this.client = axios.create({ | |
| baseURL, |
| this.client = axios.create({ | ||
| baseURL: `${env.FS_BASE_URL}/api/${env.FS_API_VERSION}`, | ||
| headers: { | ||
| 'token': env.FS_API_TOKEN, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| httpsAgent, | ||
| }); |
There was a problem hiding this comment.
httpsAgent is always included in the axios config object, even when it’s undefined. This changes the shape of the config and can break deep-equality assertions (and is noisier than necessary). Prefer conditionally adding the httpsAgent property only when you actually create an agent.
| this.client = axios.create({ | |
| baseURL: `${env.FS_BASE_URL}/api/${env.FS_API_VERSION}`, | |
| headers: { | |
| 'token': env.FS_API_TOKEN, | |
| 'Content-Type': 'application/json', | |
| }, | |
| httpsAgent, | |
| }); | |
| const axiosConfig = { | |
| baseURL: `${env.FS_BASE_URL}/api/${env.FS_API_VERSION}`, | |
| headers: { | |
| 'token': env.FS_API_TOKEN, | |
| 'Content-Type': 'application/json', | |
| }, | |
| }; | |
| if (httpsAgent) { | |
| (axiosConfig as any).httpsAgent = httpsAgent; | |
| } | |
| this.client = axios.create(axiosConfig); |
| constructor() { | ||
| const httpsAgent = process.env.NODE_TLS_REJECT_UNAUTHORIZED === '0' | ||
| ? new https.Agent({ rejectUnauthorized: false }) | ||
| : undefined; | ||
|
|
||
| this.client = axios.create({ | ||
| baseURL: `${env.FS_BASE_URL}/api/${env.FS_API_VERSION}`, | ||
| headers: { | ||
| 'token': env.FS_API_TOKEN, | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| httpsAgent, |
There was a problem hiding this comment.
This introduces new behavior (conditionally creating an https.Agent and passing it to axios) but there’s no corresponding unit test to verify the enabled/disabled paths. Add constructor-level tests for both cases (default: no httpsAgent; with NODE_TLS_REJECT_UNAUTHORIZED='0': httpsAgent configured) to prevent regressions.
Estoy usando servBay en localhost con certificados SSL locales, he tenido que añadir soporte https para poder trabajar con el MCP, intente usar mi url con http o https el error es el mismo, el certificado local no es válido y entonces no funciona. Por favor considera aceptar este PR, o si ve algo que es mejor cambiar o probar dígame.