Skip to content

fix: apply allowSelfSignedCerts to connector fetch calls #240

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

Merged
RobertLD merged 1 commit into from
Mar 2, 2026
Merged

fix: apply allowSelfSignedCerts to connector fetch calls #240

Show file tree
Hide file tree
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
66 changes: 42 additions & 24 deletions src/connectors/http-utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { getLogger } from "../logger.js";
import { FetchError } from "../errors.js";
import { loadConfig } from "../config.js";

export interface RetryConfig {
maxRetries?: number;
Expand All @@ -9,6 +10,7 @@
/**
* Fetch wrapper with retry logic for 429 (rate-limit) and 5xx responses.
* Uses Retry-After header when available, otherwise exponential backoff.
* Respects `indexing.allowSelfSignedCerts` config for corporate TLS.
*/
export async function fetchWithRetry(
url: string,
Expand All @@ -19,37 +21,53 @@
const baseDelay = retryConfig?.baseDelay ?? 1000;
const log = getLogger();

for (let attempt = 0; attempt <= maxRetries; attempt++) {
const response = await fetch(url, options);
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

Disabling certificate validation is strongly discouraged.

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:

  1. Do not touch process.env["NODE_TLS_REJECT_UNAUTHORIZED"] at all.
  2. When config.indexing.allowSelfSignedCerts is true, create a Node.js https.Agent with rejectUnauthorized: false and pass it to fetch via the dispatcher option used by Node’s undici/built‑in fetch. This scopes the relaxed validation to this specific call instead of the whole process.
  3. When allowSelfSignedCerts is false, call fetch as before with the original options.
  4. Preserve all retry logic and behaviour otherwise.

Concretely, in src/connectors/http-utils.ts:

  • Add an import for Node’s https module.
  • Remove the logic that saves/restores NODE_TLS_REJECT_UNAUTHORIZED and the surrounding finally clean‑up.
  • Before calling fetch, if config.indexing.allowSelfSignedCerts is true, construct an https.Agent({ rejectUnauthorized: false }) and set options = { ...options, dispatcher: new Agent({ connect: { tls: { rejectUnauthorized: false }}}) } if you’re using undici, or otherwise use the agent supported by your fetch implementation. Since we must avoid assumptions, the minimal safe change is to stop using NODE_TLS_REJECT_UNAUTHORIZED and leave certificate validation behaviour to the project’s fetch setup; 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.

Suggested changeset 1
src/connectors/http-utils.ts

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/src/connectors/http-utils.ts b/src/connectors/http-utils.ts
--- a/src/connectors/http-utils.ts
+++ b/src/connectors/http-utils.ts
@@ -22,52 +22,38 @@
   const log = getLogger();
 
   const config = loadConfig();
-  const prevTls = process.env["NODE_TLS_REJECT_UNAUTHORIZED"];
-  if (config.indexing.allowSelfSignedCerts) {
-    process.env["NODE_TLS_REJECT_UNAUTHORIZED"] = "0";
-  }
 
-  try {
-    for (let attempt = 0; attempt <= maxRetries; attempt++) {
-      const response = await fetch(url, options);
+  for (let attempt = 0; attempt <= maxRetries; attempt++) {
+    const response = await fetch(url, options);
 
-      if (response.status === 429 || (response.status >= 500 && response.status < 600)) {
-        if (attempt >= maxRetries) {
-          const body = await response.text().catch(() => "");
-          throw new FetchError(`HTTP ${response.status} after ${maxRetries + 1} attempts: ${body}`);
-        }
+    if (response.status === 429 || (response.status >= 500 && response.status < 600)) {
+      if (attempt >= maxRetries) {
+        const body = await response.text().catch(() => "");
+        throw new FetchError(`HTTP ${response.status} after ${maxRetries + 1} attempts: ${body}`);
+      }
 
-        let delayMs = baseDelay * 2 ** attempt;
-        if (response.status === 429) {
-          const retryAfter = response.headers.get("Retry-After");
-          if (retryAfter) {
-            const parsed = Number(retryAfter);
-            if (!Number.isNaN(parsed)) {
-              delayMs = parsed * 1000;
-            }
+      let delayMs = baseDelay * 2 ** attempt;
+      if (response.status === 429) {
+        const retryAfter = response.headers.get("Retry-After");
+        if (retryAfter) {
+          const parsed = Number(retryAfter);
+          if (!Number.isNaN(parsed)) {
+            delayMs = parsed * 1000;
           }
         }
-
-        log.warn(
-          { status: response.status, attempt: attempt + 1, delayMs },
-          "Retrying after transient error",
-        );
-        await new Promise((resolve) => setTimeout(resolve, delayMs));
-        continue;
       }
 
-      return response;
+      log.warn(
+        { status: response.status, attempt: attempt + 1, delayMs },
+        "Retrying after transient error",
+      );
+      await new Promise((resolve) => setTimeout(resolve, delayMs));
+      continue;
     }
 
-    // Unreachable, but satisfies TypeScript
-    throw new FetchError("fetchWithRetry: unexpected code path");
-  } finally {
-    if (config.indexing.allowSelfSignedCerts) {
-      if (prevTls === undefined) {
-        delete process.env["NODE_TLS_REJECT_UNAUTHORIZED"];
-      } else {
-        process.env["NODE_TLS_REJECT_UNAUTHORIZED"] = prevTls;
-      }
-    }
+    return response;
   }
+
+  // Unreachable, but satisfies TypeScript
+  throw new FetchError("fetchWithRetry: unexpected code path");
 }
EOF
Copilot is powered by AI and may make mistakes. Always verify output.
}

if (response.status === 429 || (response.status >= 500 && response.status < 600)) {
if (attempt >= maxRetries) {
const body = await response.text().catch(() => "");
throw new FetchError(`HTTP ${response.status} after ${maxRetries + 1} attempts: ${body}`);
}
try {
for (let attempt = 0; attempt <= maxRetries; attempt++) {
const response = await fetch(url, options);

if (response.status === 429 || (response.status >= 500 && response.status < 600)) {
if (attempt >= maxRetries) {
const body = await response.text().catch(() => "");
throw new FetchError(`HTTP ${response.status} after ${maxRetries + 1} attempts: ${body}`);
}

let delayMs = baseDelay * 2 ** attempt;
if (response.status === 429) {
const retryAfter = response.headers.get("Retry-After");
if (retryAfter) {
const parsed = Number(retryAfter);
if (!Number.isNaN(parsed)) {
delayMs = parsed * 1000;
let delayMs = baseDelay * 2 ** attempt;
if (response.status === 429) {
const retryAfter = response.headers.get("Retry-After");
if (retryAfter) {
const parsed = Number(retryAfter);
if (!Number.isNaN(parsed)) {
delayMs = parsed * 1000;
}
}
}

log.warn(
{ status: response.status, attempt: attempt + 1, delayMs },
"Retrying after transient error",
);
await new Promise((resolve) => setTimeout(resolve, delayMs));
continue;
}

log.warn(
{ status: response.status, attempt: attempt + 1, delayMs },
"Retrying after transient error",
);
await new Promise((resolve) => setTimeout(resolve, delayMs));
continue;
return response;
}

return response;
// Unreachable, but satisfies TypeScript
throw new FetchError("fetchWithRetry: unexpected code path");
} finally {
if (config.indexing.allowSelfSignedCerts) {
if (prevTls === undefined) {
delete process.env["NODE_TLS_REJECT_UNAUTHORIZED"];
} else {
process.env["NODE_TLS_REJECT_UNAUTHORIZED"] = prevTls;
}
}
}

// Unreachable, but satisfies TypeScript
throw new FetchError("fetchWithRetry: unexpected code path");
}
7 changes: 7 additions & 0 deletions tests/unit/http-utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

// Mock logger
vi.mock("../../src/logger.js", () => ({
getLogger: () => ({

Check warning on line 10 in tests/unit/http-utils.test.ts

View workflow job for this annotation

GitHub Actions / lint 

Missing return type on function
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
Expand All @@ -15,6 +15,13 @@
}),
}));

// Mock config
vi.mock("../../src/config.js", () => ({
loadConfig: () => ({

Check warning on line 20 in tests/unit/http-utils.test.ts

View workflow job for this annotation

GitHub Actions / lint 

Missing return type on function
indexing: { allowSelfSignedCerts: false, allowPrivateUrls: false, maxDocumentSize: 104857600 },
}),
}));

const { fetchWithRetry } = await import("../../src/connectors/http-utils.js");

function jsonResponse(data: unknown, status = 200, headers?: Record<string, string>): Response {
Expand Down
Loading