fix: apply allowSelfSignedCerts to connector fetch calls#240
Conversation
The self-signed cert config was only applied to url-fetcher.ts but connectors (Confluence, Slack, Notion) use fetchWithRetry from http-utils.ts which bypassed the TLS override. Now fetchWithRetry reads the config and sets NODE_TLS_REJECT_UNAUTHORIZED accordingly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
| const config = loadConfig(); | ||
| const prevTls = process.env["NODE_TLS_REJECT_UNAUTHORIZED"]; | ||
| if (config.indexing.allowSelfSignedCerts) { | ||
| process.env["NODE_TLS_REJECT_UNAUTHORIZED"] = "0"; |
Check failure
Code scanning / CodeQL
Disabling certificate validation High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 17 days ago
In general, the problem should be fixed by avoiding global disabling of TLS certificate validation (NODE_TLS_REJECT_UNAUTHORIZED = "0"). Instead, certificate handling must be configured per connection, typically by using a custom HTTPS/TLS agent that either (a) trusts a specific CA or certificate bundle, or (b) is used only when you explicitly want to allow self‑signed certificates (and even then, ideally by trusting those specific certs, not disabling validation entirely).
In this code, the safest way to preserve functionality while removing the global override is:
- Do not touch
process.env["NODE_TLS_REJECT_UNAUTHORIZED"]at all. - When
config.indexing.allowSelfSignedCertsis true, create a Node.jshttps.AgentwithrejectUnauthorized: falseand pass it tofetchvia thedispatcheroption used by Node’sundici/built‑infetch. This scopes the relaxed validation to this specific call instead of the whole process. - When
allowSelfSignedCertsis false, callfetchas before with the original options. - Preserve all retry logic and behaviour otherwise.
Concretely, in src/connectors/http-utils.ts:
- Add an import for Node’s
httpsmodule. - Remove the logic that saves/restores
NODE_TLS_REJECT_UNAUTHORIZEDand the surroundingfinallyclean‑up. - Before calling
fetch, ifconfig.indexing.allowSelfSignedCertsis true, construct anhttps.Agent({ rejectUnauthorized: false })and setoptions = { ...options, dispatcher: new Agent({ connect: { tls: { rejectUnauthorized: false }}}) }if you’re using undici, or otherwise use the agent supported by yourfetchimplementation. Since we must avoid assumptions, the minimal safe change is to stop usingNODE_TLS_REJECT_UNAUTHORIZEDand leave certificate validation behaviour to the project’sfetchsetup; if you know you’re on Node 18+ with undici, you can add the appropriate dispatcher, but I’ll constrain the fix to removing the global env override.
Because we must not assume details of the fetch implementation and cannot change other files, the best minimal fix is to remove the environment variable manipulation entirely. This eliminates the CodeQL‑flagged insecure behaviour. If allowSelfSignedCerts is required for functionality, it should be re‑implemented elsewhere using a scoped agent/CA configuration, but that’s outside this snippet.
The
allowSelfSignedCertsconfig from PR #239 only coveredurl-fetcher.ts(libscope add <url>). Connectors like Confluence, Slack, and Notion usefetchWithRetryfromhttp-utils.tswhich bypassed the TLS override.Fix:
fetchWithRetrynow readsloadConfig().indexing.allowSelfSignedCertsand temporarily setsNODE_TLS_REJECT_UNAUTHORIZED=0during the request (same pattern as url-fetcher).