Skip to content

feat: added https data protection via package#7537

Open
jasonsaayman wants to merge 1 commit intov1.xfrom
fix/6320-https-data-sent-in-cleartext-to-proxy
Open

feat: added https data protection via package#7537
jasonsaayman wants to merge 1 commit intov1.xfrom
fix/6320-https-data-sent-in-cleartext-to-proxy

Conversation

@jasonsaayman
Copy link
Copy Markdown
Member

@jasonsaayman jasonsaayman commented Mar 20, 2026

Closes #6320


Summary by cubic

Secure HTTPS requests over HTTP proxies by tunneling via https-proxy-agent, preventing plaintext leakage of HTTPS payloads. Fixes #6320.

Description

  • Summary of changes

    • Detect HTTPS targets behind HTTP proxies and use CONNECT via HttpsProxyAgent.
    • Preserve TLS options from httpsAgent (e.g., rejectUnauthorized, ca, cert) when tunneling.
    • Remove forwarding of Proxy-Authorization headers to origins; auth is handled in CONNECT.
    • Ensure redirects reapply proxy and agent options.
    • Added dependency: https-proxy-agent@^8.0.0.
  • Reasoning

    • Previously, HTTPS requests through HTTP proxies could be sent in cleartext. Tunneling fixes this and aligns with proxy best practices.
  • Additional context

    • Behavior for HTTP targets or HTTPS proxies is unchanged.
    • Env-based proxy resolution via proxy-from-env still applies.

Docs

  • Document that HTTPS requests over HTTP proxies now use CONNECT tunneling automatically.
  • Proxy auth: send credentials via proxy.auth or proxy: "http://user:pass@host:port".
  • TLS config remains on httpsAgent; Axios carries these options through the tunnel.
  • No migration steps expected.

Testing

  • Added unit tests (http adapter):
    • Uses CONNECT for HTTPS via HTTP proxy.
    • Sends Proxy-Authorization on CONNECT for authenticated proxies.
    • Uses CONNECT during HTTPS redirects via HTTP proxy.
  • Updated fetch adapter tests to create per-test Axios instances (removed shared baseURL usage).
  • Coverage verifies security behavior; no additional tests needed.

Written for commit 7320936. Summary will update on new commits.

@jasonsaayman jasonsaayman self-assigned this Mar 20, 2026
@jasonsaayman jasonsaayman added priority::high A high priority issue commit::fix The PR is related to a bugfix labels Mar 20, 2026
protocol: 'http:',
},
httpsAgent: new https.Agent({
rejectUnauthorized: false,

Check failure

Code scanning / CodeQL

Disabling certificate validation High test

Disabling certificate validation is strongly discouraged.

Copilot Autofix

AI 20 days ago

In general, instead of disabling certificate validation (rejectUnauthorized: false), configure the HTTPS client to trust the specific certificate or certificate authority used by the server. This preserves TLS security while allowing tests to work with self‑signed or test certificates.

In this specific test, we should remove rejectUnauthorized: false and supply appropriate trust material to the https.Agent. Since the test already reads cert.pem (the server certificate), we can pass that certificate (or its issuing CA certificate, if available) via the ca option on the https.Agent. This preserves certificate validation but makes the self‑signed or non‑public certificate acceptable to the client.

Concretely, in tests/unit/adapters/http.test.js within the it('should use CONNECT tunnel for HTTPS target via HTTP proxy', ...) block:

  • Keep the existing tlsOptions used to create the HTTPS server.
  • Modify the httpsAgent configuration passed to axios.get:
    • Remove rejectUnauthorized: false.
    • Add a ca property whose value is the certificate data read from cert.pem. We already read this into tlsOptions.cert; we can reuse that value when creating the https.Agent.
  • This change requires no new imports; we will just pass tlsOptions.cert into the agent.

Functionality remains the same in terms of behavior under test (CONNECT tunneling and response body), but now the TLS connection uses proper certificate validation against the provided CA/cert.

Suggested changeset 1
tests/unit/adapters/http.test.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/adapters/http.test.js b/tests/unit/adapters/http.test.js
--- a/tests/unit/adapters/http.test.js
+++ b/tests/unit/adapters/http.test.js
@@ -1389,7 +1389,7 @@
           protocol: 'http:',
         },
         httpsAgent: new https.Agent({
-          rejectUnauthorized: false,
+          ca: tlsOptions.cert,
         }),
       });
 
EOF
@@ -1389,7 +1389,7 @@
protocol: 'http:',
},
httpsAgent: new https.Agent({
rejectUnauthorized: false,
ca: tlsOptions.cert,
}),
});

Copilot is powered by AI and may make mistakes. Always verify output.
},
},
httpsAgent: new https.Agent({
rejectUnauthorized: false,

Check failure

Code scanning / CodeQL

Disabling certificate validation High test

Disabling certificate validation is strongly discouraged.

Copilot Autofix

AI 20 days ago

In general, instead of disabling certificate validation with rejectUnauthorized: false, configure the HTTPS client to trust the specific certificate(s) used by the test server. For Node.js, this is done by passing the server’s certificate (or a CA certificate that signed it) via the ca option to https.Agent, while leaving rejectUnauthorized at its default true.

Concretely in tests/unit/adapters/http.test.js, we already read the test key and cert into tlsOptions for https.createServer. We can reuse the same cert.pem content as a trusted CA for the client side. Replace the httpsAgent construction at lines 1456–1458:

httpsAgent: new https.Agent({
  rejectUnauthorized: false,
}),

with:

httpsAgent: new https.Agent({
  ca: fs.readFileSync(path.join(adaptersTestsDir, 'cert.pem')),
}),

This keeps certificate validation enabled but tells the client to trust the self-signed certificate from cert.pem, so the request to https://localhost:... still succeeds and the rest of the test logic remains unchanged. No new imports are needed; fs and path are already imported, and adaptersTestsDir is assumed to be defined earlier in the same file (as the existing code already uses it to read the key and cert).

Suggested changeset 1
tests/unit/adapters/http.test.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/adapters/http.test.js b/tests/unit/adapters/http.test.js
--- a/tests/unit/adapters/http.test.js
+++ b/tests/unit/adapters/http.test.js
@@ -1454,7 +1454,7 @@
           },
         },
         httpsAgent: new https.Agent({
-          rejectUnauthorized: false,
+          ca: fs.readFileSync(path.join(adaptersTestsDir, 'cert.pem')),
         }),
       });
 
EOF
@@ -1454,7 +1454,7 @@
},
},
httpsAgent: new https.Agent({
rejectUnauthorized: false,
ca: fs.readFileSync(path.join(adaptersTestsDir, 'cert.pem')),
}),
});

Copilot is powered by AI and may make mistakes. Always verify output.
protocol: 'http:',
},
httpsAgent: new https.Agent({
rejectUnauthorized: false,

Check failure

Code scanning / CodeQL

Disabling certificate validation High test

Disabling certificate validation is strongly discouraged.

Copilot Autofix

AI 20 days ago

In general, the fix is to avoid disabling TLS certificate validation and instead configure the client to trust the expected certificate (or a specific CA) while keeping rejectUnauthorized at its secure default (true). For Node’s https.Agent, this usually means removing rejectUnauthorized: false and, if you are using a self‑signed or test certificate, providing the certificate (or CA) via the ca option, so the handshake remains authenticated.

For this specific test, the best fix with minimal behavioral change is:

  • Remove rejectUnauthorized: false from the https.Agent options.
  • Add a ca option pointing to the same certificate used to start httpsTargetServer (cert.pem), so the agent will trust that server’s certificate while still rejecting others.

Concretely, in tests/unit/adapters/http.test.js in the "should use CONNECT tunnel for HTTPS redirect via HTTP proxy" test:

  • Replace the httpsAgent: new https.Agent({ rejectUnauthorized: false, }) block with httpsAgent: new https.Agent({ ca: fs.readFileSync(path.join(adaptersTestsDir, 'cert.pem')), }).

No new imports are required: fs, path, and adaptersTestsDir are already in use in this file (the snippet already reads the same cert.pem and key.pem into tlsOptions).

Suggested changeset 1
tests/unit/adapters/http.test.js

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/tests/unit/adapters/http.test.js b/tests/unit/adapters/http.test.js
--- a/tests/unit/adapters/http.test.js
+++ b/tests/unit/adapters/http.test.js
@@ -1541,7 +1541,7 @@
           protocol: 'http:',
         },
         httpsAgent: new https.Agent({
-          rejectUnauthorized: false,
+          ca: fs.readFileSync(path.join(adaptersTestsDir, 'cert.pem')),
         }),
       });
 
EOF
@@ -1541,7 +1541,7 @@
protocol: 'http:',
},
httpsAgent: new https.Agent({
rejectUnauthorized: false,
ca: fs.readFileSync(path.join(adaptersTestsDir, 'cert.pem')),
}),
});

Copilot is powered by AI and may make mistakes. Always verify output.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 5 files

Confidence score: 3/5

  • There is a concrete runtime risk in lib/adapters/http.js: building the proxy URL with ${proxy.port} can trigger TypeError: Invalid URL when users provide a proxy config without a port.
  • Given the medium severity (6/10) and high confidence (8/10), this is a meaningful user-facing failure path rather than a cosmetic issue, so merge risk is moderate.
  • Pay close attention to lib/adapters/http.js - proxy URL construction should omit the port segment when proxy.port is undefined.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="lib/adapters/http.js">

<violation number="1" location="lib/adapters/http.js:232">
P2: Constructing the proxy URL with `proxy.port` via string interpolation will throw `TypeError: Invalid URL` when `proxy.port` is `undefined` (user-provided proxy config without an explicit port). Conditionally include the port segment.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

const proxyUrl = `http://${encodeURIComponent(username)}:${encodeURIComponent(password)}@${proxyHost}:${proxy.port}`;
options.agent = new HttpsProxyAgent(proxyUrl, agentOptions);
} else {
options.agent = new HttpsProxyAgent(`http://${proxyHost}:${proxy.port}`, agentOptions);
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 20, 2026

Choose a reason for hiding this comment

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

P2: Constructing the proxy URL with proxy.port via string interpolation will throw TypeError: Invalid URL when proxy.port is undefined (user-provided proxy config without an explicit port). Conditionally include the port segment.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/adapters/http.js, line 232:

<comment>Constructing the proxy URL with `proxy.port` via string interpolation will throw `TypeError: Invalid URL` when `proxy.port` is `undefined` (user-provided proxy config without an explicit port). Conditionally include the port segment.</comment>

<file context>
@@ -210,28 +211,65 @@ function setProxy(options, configProxy, location) {
+        const proxyUrl = `http://${encodeURIComponent(username)}:${encodeURIComponent(password)}@${proxyHost}:${proxy.port}`;
+        options.agent = new HttpsProxyAgent(proxyUrl, agentOptions);
+      } else {
+        options.agent = new HttpsProxyAgent(`http://${proxyHost}:${proxy.port}`, agentOptions);
+      }
+
</file context>
Suggested change
options.agent = new HttpsProxyAgent(`http://${proxyHost}:${proxy.port}`, agentOptions);
options.agent = new HttpsProxyAgent(`http://${proxyHost}${proxy.port ? ':' + proxy.port : ''}`, agentOptions);
Fix with Cubic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit::fix The PR is related to a bugfix priority::high A high priority issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: Axios sends HTTPS data in cleartext to a proxy (regression)

2 participants