Skip to content

feat(client): add HTTPS agent configuration for secure requests #3

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daniel89fg wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(client): add HTTPS agent configuration for secure requests #3

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/fs/client.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,22 @@
import axios, { AxiosInstance } from 'axios';
import https from 'https';
import { env } from '../env.js';

export class FacturaScriptsClient {
private client: AxiosInstance;

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}`,
Comment on lines +9 to 14
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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,

Copilot uses AI. Check for mistakes.
headers: {
'token': env.FS_API_TOKEN,
'Content-Type': 'application/json',
},
httpsAgent,
Comment on lines 8 to +19
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
});
Comment on lines 13 to 20
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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);

Copilot uses AI. Check for mistakes.
}

Expand Down
Loading