Create PQC Compliance Integration Test for Gaxios - #9016
Conversation
Adds a comprehensive integration test in core/packages/gaxios/test/test.pqc.ts that ensures Gaxios successfully negotiates TLS connections using the hybrid post-quantum key exchange group X25519MLKEM768 when running on PQC-compliant Node.js runtimes. The test utilizes a native, in-memory HTTPS server using pre-generated RSA key/certificate assets, bypasses nock intercepts on localhost, and includes a graceful runtime-compatibility guard. Co-authored-by: danieljbruce <8935272+danieljbruce@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request formats type definitions and test files, and introduces a new integration test suite to verify Post-Quantum Cryptography (PQC) compliance using the hybrid curve X25519MLKEM768. The review feedback recommends simplifying the PQC support check by removing fragile Node.js version checks in favor of pure feature detection, and properly closing the test HTTPS server by terminating active connections and awaiting its closure to prevent potential test hangs.
| before(function () { | ||
| const versionStr = process.versions.node; | ||
| const [major, minor] = versionStr.split('.').map(Number); | ||
|
|
||
| const isSupportedVersion = | ||
| (major === 22 && minor >= 20) || | ||
| (major === 24 && minor >= 7) || | ||
| major > 24; | ||
|
|
||
| let isCurveSupported = false; | ||
| try { | ||
| tls.createSecureContext({ecdhCurve: 'X25519MLKEM768'}); | ||
| isCurveSupported = true; | ||
| } catch (e) { | ||
| isCurveSupported = false; | ||
| } | ||
|
|
||
| isPQCSupported = isSupportedVersion && isCurveSupported; | ||
|
|
||
| if (!isPQCSupported) { | ||
| console.log( | ||
| `Skipping PQC Integration test on unsupported Node.js runtime version: ${versionStr}`, | ||
| ); | ||
| this.skip(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Relying on hardcoded Node.js version checks (like checking for specific major/minor versions) is fragile and can easily break or incorrectly skip tests on other supported runtimes (such as Node.js v23, custom builds, or future releases). Since you are already performing a robust feature detection check using tls.createSecureContext({ecdhCurve: 'X25519MLKEM768'}), you can completely remove the version parsing logic and rely solely on the feature check.
before(function () {
try {
tls.createSecureContext({ecdhCurve: 'X25519MLKEM768'});
isPQCSupported = true;
} catch (e) {
isPQCSupported = false;
}
if (!isPQCSupported) {
console.log(
'Skipping PQC Integration test on unsupported Node.js runtime version: ' + process.versions.node,
);
this.skip();
}
});| } finally { | ||
| server.close(); | ||
| } |
There was a problem hiding this comment.
The server.close() method is asynchronous and does not force-close active connections by default. If there are any active or keep-alive connections, the server will remain open, which can cause the test runner to hang or lead to "port in use" errors in subsequent tests. Since this test runs on Node.js v22+, you can use server.closeAllConnections() and await the server closure to ensure a clean teardown.
} finally {
if (typeof server.closeAllConnections === 'function') {
server.closeAllConnections();
}
await new Promise<void>(resolve => server.close(() => resolve()));
}Adds a comprehensive integration test in core/packages/gaxios/test/test.pqc.ts that ensures Gaxios successfully negotiates TLS connections using the hybrid post-quantum key exchange group X25519MLKEM768 when running on PQC-compliant Node.js runtimes. The test utilizes a native, in-memory HTTPS server using pre-generated RSA key/certificate assets, bypasses nock intercepts on localhost, and includes a graceful runtime-compatibility guard. Co-authored-by: danieljbruce <8935272+danieljbruce@users.noreply.github.com>
This PR introduces a robust Post-Quantum Cryptography (PQC) integration test for Gaxios. It verifies that standard HTTPS requests made with Gaxios successfully negotiate TLS connections using the hybrid post-quantum key exchange group X25519MLKEM768 when executed on a PQC-compliant Node.js runtime (v22.20+ and v24.7.0+).
Key elements implemented:
'X25519MLKEM768'to safely skip on unsupported systems.https.Agentoverride to hook client-sideTLSSocketgeneration, verifying correct protocol (TLSv1.3) and negotiation.PR created automatically by Jules for task 5397569580220749849 started by @danieljbruce