Commit 519c65ec6a1 for nodejs
commit 519c65ec6a1c9d4553b6bcac2144f511c8a3c8bd
Author: Matteo Collina <hello@matteocollina.com>
Date: Thu Oct 8 10:36:05 2026 +0200
http: redact proxy credentials in invalid proxy URL errors
The earlier fix to redact proxy credentials only covered the tunnel
error and the ProxyConfig.href property. Four other error paths still
interpolate the raw proxy URL, credentials included, into
ERR_PROXY_INVALID_CONFIG messages:
* lib/internal/http.js - ProxyConfig constructor when new URL() fails.
* lib/internal/http.js - parseProxyUrl when the URL contains CR or LF.
* lib/http.js - setGlobalProxyFromEnv() with an unparseable http_proxy.
* lib/http.js - setGlobalProxyFromEnv() with an unparseable https_proxy
Add a redactProxyUrl() helper that strips userinfo when parsing
succeeds and falls back to a textual strip for the unparseable cases
that reach the vulnerable paths, then route all four sites through it.
Refs: https://hackerone.com/reports/3966924
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: https://github.com/nodejs-private/node-private/pull/1001
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
CVE-ID: CVE-2026-123871
diff --git a/lib/http.js b/lib/http.js
index bc99ba61e37..142d8b98fcf 100644
--- a/lib/http.js
+++ b/lib/http.js
@@ -58,6 +58,7 @@ const {
const {
parseProxyUrl,
getGlobalAgent,
+ redactProxyUrl,
} = require('internal/http');
const { URL } = require('internal/url');
let maxHeaderSize;
@@ -153,10 +154,10 @@ function setGlobalProxyFromEnv(env = process.env) {
}
if (httpProxy && !URL.canParse(httpProxy)) {
- throw new ERR_PROXY_INVALID_CONFIG(httpProxy);
+ throw new ERR_PROXY_INVALID_CONFIG(redactProxyUrl(httpProxy));
}
if (httpsProxy && !URL.canParse(httpsProxy)) {
- throw new ERR_PROXY_INVALID_CONFIG(httpsProxy);
+ throw new ERR_PROXY_INVALID_CONFIG(redactProxyUrl(httpsProxy));
}
let originalDispatcher, originalHttpsAgent, originalHttpAgent;
diff --git a/lib/internal/http.js b/lib/internal/http.js
index 999ade29ed0..68d4a75a253 100644
--- a/lib/internal/http.js
+++ b/lib/internal/http.js
@@ -112,13 +112,40 @@ function ipToInt(ip) {
* @property {Array<string>} bypassList - List of hosts to bypass the proxy.
* @property {object} proxyConnectionOptions - Options for connecting to the proxy server.
*/
+
+/**
+ * Redacts the userinfo (username/password) from a proxy URL so that credentials
+ * never surface in error messages, which can end up in logs and crash reports.
+ * This must not depend on parsing succeeding: it is used precisely when the URL
+ * failed to parse.
+ * @param {string} proxyUrl - The proxy URL to redact.
+ * @returns {string} The proxy URL with any userinfo removed.
+ */
+function redactProxyUrl(proxyUrl) {
+ const raw = `${proxyUrl}`;
+ try {
+ const parsedURL = new URL(raw);
+ if (parsedURL.username || parsedURL.password) {
+ parsedURL.username = '';
+ parsedURL.password = '';
+ return parsedURL.href;
+ }
+ return raw;
+ } catch {
+ // Unparseable: strip the userinfo textually. Greedy up to the last '@' in the
+ // authority so a password containing '@' is redacted whole rather than
+ // leaving a trailing fragment.
+ return raw.replace(/\/\/[^/]*@/, '//[redacted]@');
+ }
+}
+
class ProxyConfig {
constructor(proxyUrl, keepAlive, noProxyList) {
let parsedURL;
try {
parsedURL = new URL(proxyUrl);
} catch {
- throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${proxyUrl}`);
+ throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${redactProxyUrl(proxyUrl)}`);
}
const { hostname, port, protocol, username, password } = parsedURL;
@@ -210,7 +237,7 @@ function parseProxyUrl(env, protocol) {
}
if (proxyUrl.includes('\r') || proxyUrl.includes('\n')) {
- throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${proxyUrl}`);
+ throw new ERR_PROXY_INVALID_CONFIG(`Invalid proxy URL: ${redactProxyUrl(proxyUrl)}`);
}
return proxyUrl;
@@ -290,4 +317,5 @@ module.exports = {
filterEnvForProxies,
getGlobalAgent,
parseProxyUrl,
+ redactProxyUrl,
};
diff --git a/test/client-proxy/test-http-proxy-redact-credentials.mjs b/test/client-proxy/test-http-proxy-redact-credentials.mjs
new file mode 100644
index 00000000000..b13088fc1a5
--- /dev/null
+++ b/test/client-proxy/test-http-proxy-redact-credentials.mjs
@@ -0,0 +1,145 @@
+// Regression test for the incomplete CVE-2026-48615 fix: proxy credentials must
+// never appear in ERR_PROXY_INVALID_CONFIG messages. Four error paths interpolate
+// the raw proxy URL (userinfo included):
+// 1. lib/internal/http.js - new URL() fails in the ProxyConfig constructor.
+// 2. lib/internal/http.js - proxy URL contains CR or LF.
+// 3. lib/http.js - http.setGlobalProxyFromEnv() with an unparseable http_proxy.
+// 4. lib/http.js - http.setGlobalProxyFromEnv() with an unparseable https_proxy.
+//
+// The well-formed credential control confirms redaction on the normal path is
+// preserved.
+
+import '../common/index.mjs';
+import assert from 'node:assert';
+import { spawnSync } from 'node:child_process';
+
+// A proxy credential that both contains '@' and yields substrings in every
+// meaningful piece that must not survive in any error message.
+const SECRET = 'S3cr3t-Pr0xy-P@ssw0rd';
+
+// Asserts that a message does not contain any substring of length >= 4 derived
+// from the secret. Length-4 substrings are enough to catch partial leaks while
+// avoiding false positives from unrelated words.
+function assertNoLeak(message, label) {
+ assert.doesNotMatch(message, /S3cr3t/, `${label}: leaked username`);
+ assert.doesNotMatch(message, /Pr0xy/, `${label}: leaked password fragment`);
+ assert.doesNotMatch(message, /ssw0rd/, `${label}: leaked password fragment`);
+ assert.doesNotMatch(message, /P@ssw0rd/, `${label}: leaked password`);
+ // A malicious proxy URL must not survive verbatim.
+ assert.notStrictEqual(message, SECRET, `${label}: leaked the whole secret`);
+}
+
+function runNode(code, env = {}) {
+ const res = spawnSync(process.execPath, ['-e', code], {
+ env: { ...process.env, ...env },
+ encoding: 'utf8',
+ });
+ return res;
+}
+
+function setGlobalProxyFromEnv(proxyEnv) {
+ return `try {
+ require('http').setGlobalProxyFromEnv(${JSON.stringify(proxyEnv)});
+ } catch (e) {
+ console.log(e.message);
+ }
+ `;
+}
+
+// --- Sites 3 & 4: public http.setGlobalProxyFromEnv(), reachable without flags ---
+
+{
+ // Site 3: unparseable http_proxy.
+ const { stdout, stderr } = runNode(setGlobalProxyFromEnv({
+ http_proxy: `http://alice:${SECRET}@ `,
+ }));
+ assertNoLeak(stdout, 'site 3');
+ assertNoLeak(stderr, 'site 3 stderr');
+ assert.match(stdout, /http:\/\/\[redacted\]@/);
+}
+
+{
+ // Site 4: unparseable https_proxy.
+ const { stdout, stderr } = runNode(setGlobalProxyFromEnv({
+ https_proxy: `http://alice:${SECRET}@::bad`,
+ }));
+ assertNoLeak(stdout, 'site 4');
+ assertNoLeak(stderr, 'site 4 stderr');
+ assert.match(stdout, /http:\/\/\[redacted\]@/);
+}
+
+// --- Site 1: ProxyConfig constructor, new URL() throws (reachable via Agent) ---
+
+{
+ const code = `try {
+ new http.Agent({
+ proxyEnv: { HTTP_PROXY: 'http://alice:${SECRET}@::bad' },
+ });
+ } catch (e) {
+ console.log(e.message);
+ }`;
+ const { stdout, stderr } = runNode(code);
+ assertNoLeak(stdout, 'site 1');
+ assertNoLeak(stderr, 'site 1 stderr');
+ assert.match(stdout, /Invalid proxy URL: http:\/\/\[redacted\]@/);
+}
+
+// --- Site 2: proxy URL containing CR or LF ---
+
+{
+ const code = `try {
+ require('http').setGlobalProxyFromEnv({
+ http_proxy: 'http://alice:${SECRET}@\\nhost',
+ });
+ } catch (e) {
+ console.log(e.message);
+ }`;
+ const { stdout, stderr } = runNode(code);
+ assertNoLeak(stdout, 'site 2');
+ assertNoLeak(stderr, 'site 2 stderr');
+}
+
+// --- Site 1 & 2 via the environment-driven, opt-in path (NODE_USE_ENV_PROXY) ---
+
+{
+ const code = `require('http').request('http://example.com/').on('error', () => {}).end()`;
+ const { stdout, stderr } = runNode(code, {
+ NODE_USE_ENV_PROXY: '1',
+ http_proxy: `http://alice:${SECRET}@:::bad`,
+ });
+ // Sites 1/2 throw during builtin module init, so stderr carries the message and
+ // the process exits non-zero. The application cannot catch it.
+ assertNoLeak(stdout, 'env-driven stdout');
+ assertNoLeak(stderr, 'env-driven stderr');
+ const { status } = runNode(code, {
+ NODE_USE_ENV_PROXY: '1',
+ http_proxy: `http://alice:${SECRET}@:::bad`,
+ });
+ assert.notStrictEqual(status, 0);
+}
+
+// --- Control: well-formed URL with credentials is redacted on the normal path ---
+
+{
+ const code = `try {
+ require('http').setGlobalProxyFromEnv({
+ http_proxy: 'http://alice:${SECRET}@localhost:8080',
+ });
+ console.log('ok');
+ } catch (e) {
+ console.log('unexpected throw: ' + e.message);
+ }`;
+ const { stdout, status } = runNode(code);
+ assert.strictEqual(status, 0);
+ assert.strictEqual(stdout.trim(), 'ok');
+}
+
+// --- Control: an unparseable URL with no credentials is left untouched ---
+
+{
+ const { stdout } = runNode(setGlobalProxyFromEnv({
+ http_proxy: 'http://host:port',
+ }));
+ assert.strictEqual(stdout.trim(), 'http://host:port');
+ assert.doesNotMatch(stdout, /\[redacted\]/);
+}