feat: allow loading root cas from global var #5
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Caution Review failedThe pull request is closed. WalkthroughThis set of changes refactors the handling of root CA certificates by removing static inclusion and parsing of additional root CAs, simplifying the main CA list to only Mozilla roots, and introducing a dynamic, global mechanism for extending root CAs at runtime. File structure and TypeScript configuration are updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant App
participant additional-root-cas.js
participant parse-certificate.ts
participant mozilla-root-cas.ts
App->>additional-root-cas.js: (On startup) Initialize TLS_ADDITIONAL_ROOT_CA_LIST
App->>parse-certificate.ts: verifyCertificateChain()
parse-certificate.ts->>mozilla-root-cas.ts: Import MOZILLA_ROOT_CA_LIST
parse-certificate.ts->>additional-root-cas.js: Access global TLS_ADDITIONAL_ROOT_CA_LIST
parse-certificate.ts->>parse-certificate.ts: loadRootCAs() (combine Mozilla + additional CAs)
parse-certificate.ts->>App: Return verification result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Possibly related PRs
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/types/index.ts (1)
6-8: Global declaration looks correct but consider runtime safety.The global declaration properly types the
TLS_ADDITIONAL_ROOT_CA_LISTvariable for TypeScript awareness. However, since this relies on runtime initialization from a JavaScript module, consider adding runtime existence checks in consuming code to prevent potentialReferenceErrors.Example of safer access pattern:
const additionalCAs = typeof TLS_ADDITIONAL_ROOT_CA_LIST !== 'undefined' ? TLS_ADDITIONAL_ROOT_CA_LIST : []package.json (1)
3-3: Consider whether this should be a minor version bump.The version increment from
0.0.5to0.0.6represents a patch-level change, but the PR introduces significant architectural changes in root CA handling. Consider whether this warrants a minor version bump (e.g.,0.1.0) if the changes could affect existing consumers or introduce new features.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
package.json(1 hunks)src/scripts/ca-template.ts(1 hunks)src/scripts/update-ca-certs.ts(1 hunks)src/types/index.ts(1 hunks)src/utils/additional-root-cas.js(1 hunks)src/utils/index.ts(1 hunks)src/utils/mozilla-root-cas.ts(2 hunks)src/utils/parse-certificate.ts(4 hunks)tsconfig.json(1 hunks)
👮 Files not reviewed due to content moderation or server errors (1)
- src/utils/mozilla-root-cas.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/utils/parse-certificate.ts (2)
src/scripts/ca-template.ts (1)
MOZILLA_ROOT_CA_LIST(6-6)src/utils/x509.ts (1)
loadX509FromPem(9-56)
src/utils/mozilla-root-cas.ts (1)
src/scripts/ca-template.ts (1)
MOZILLA_ROOT_CA_LIST(6-6)
🔇 Additional comments (6)
tsconfig.json (1)
20-21: LGTM! Improved file inclusion patterns.The changes correctly expand the TypeScript compilation scope:
"src/**/*.ts"now recursively includes all TypeScript files (improvement over previous"src/*/*.ts")"src/**/*.js"inclusion is necessary for the newadditional-root-cas.jsmodule mentioned in the PRThese changes align well with the existing
allowJs: trueconfiguration.src/scripts/update-ca-certs.ts (1)
36-36: LGTM! Output path correctly updated.The change from
src/utils/root-ca.tstosrc/utils/mozilla-root-cas.tsaligns well with the architectural refactor to separate Mozilla root CAs from additional CAs. The script logic remains unchanged, focusing only on Mozilla certificates as intended.src/utils/index.ts (1)
14-14: LGTM! Export path correctly updated.The export source update from
./root-cato./mozilla-root-casis necessary and correct following the file rename in the CA certificate generation script. This maintains proper module exports while supporting the new architectural separation.src/utils/parse-certificate.ts (3)
1-1: LGTM: Import changes support the new dynamic root CA loading architecture.The imports correctly add the necessary dependencies:
- Side-effect module for global initialization
- Mozilla root CA list for the primary certificate source
- PEM parser for certificate processing
Also applies to: 7-7, 9-9
19-19: LGTM: Proper caching implementation for root CAs.The module-scoped
ROOT_CASvariable correctly implements lazy loading with caching to avoid re-parsing certificates on subsequent calls.
165-166: LGTM: Correctly implements dynamic root CA loading.The function now uses
loadRootCAs()for lazy initialization while maintaining the same API contract and combining both Mozilla and additional root CAs.
| */ | ||
| export const ROOT_CAS = ROOT_CA_LIST | ||
| .map(loadX509FromPem) | ||
| export const MOZILLA_ROOT_CA_LIST = ['<<CERTIFICATES>>'] |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify that the build process correctly replaces the placeholder.
The export now uses a placeholder ['<<CERTIFICATES>>'] which suggests this file is processed during the build. Ensure that:
- The build script properly replaces the placeholder with actual Mozilla root CA certificates
- The placeholder format is consistently handled across the build pipeline
- Error handling exists if the placeholder isn't replaced
🏁 Script executed:
#!/bin/bash
# Description: Verify build process handles the ca-template correctly
# Expected: Find build scripts that process this template
# Search for build scripts that might process the template
rg -A 5 -B 5 "<<CERTIFICATES>>" --type ts --type js --type json
# Look for references to ca-template in build configurations
rg -A 3 "ca-template" --type ts --type js --type jsonLength of output: 1267
Build script correctly replaces the placeholder in ca-template.ts
The src/scripts/update-ca-certs.ts script reads src/scripts/ca-template.ts, replaces the '<\<CERTIFICATES>>' placeholder with the generated PEM blocks, and writes the result to src/utils/mozilla-root-cas.ts. No further changes are needed to the template itself.
Action required:
- Enhance
src/scripts/update-ca-certs.tswith a sanity check that the placeholder was actually replaced. For example, after doingthrow an error ifconst newData = data.replace('\'<<CERTIFICATES>>\'', certs);
newData === data, to fail fast if the placeholder wasn’t found.
🤖 Prompt for AI Agents
In src/scripts/update-ca-certs.ts, after replacing the placeholder string
"'<<CERTIFICATES>>'" with the generated PEM blocks, add a check to verify that
the replacement actually occurred by comparing the new string with the original.
If they are identical, throw an error to fail fast and indicate the placeholder
was not found. This ensures the build script correctly updates the template.
Comment on lines
+1
to
+2
| /* eslint indent: 0 */ | ||
| global.TLS_ADDITIONAL_ROOT_CA_LIST ||= [] |
There was a problem hiding this comment.
Review the security implications of using global variables for root CAs.
Using a global variable to manage root certificate authorities introduces potential security and reliability concerns:
- Global state mutation: Multiple modules can modify
TLS_ADDITIONAL_ROOT_CA_LIST, potentially leading to race conditions or unintended certificate additions - Lack of access control: No validation or authorization mechanism prevents malicious code from adding compromised certificates
Consider implementing a more controlled approach with proper encapsulation and validation.
-/* eslint indent: 0 */
-global.TLS_ADDITIONAL_ROOT_CA_LIST ||= []
+/* eslint indent: 0 */
+// Initialize global CA list with proper validation
+if (!global.TLS_ADDITIONAL_ROOT_CA_LIST) {
+ global.TLS_ADDITIONAL_ROOT_CA_LIST = []
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /* eslint indent: 0 */ | |
| global.TLS_ADDITIONAL_ROOT_CA_LIST ||= [] | |
| /* eslint indent: 0 */ | |
| // Initialize global CA list with proper validation | |
| if (!global.TLS_ADDITIONAL_ROOT_CA_LIST) { | |
| global.TLS_ADDITIONAL_ROOT_CA_LIST = [] | |
| } |
🤖 Prompt for AI Agents
In src/utils/additional-root-cas.js at lines 1 to 2, avoid using a global
variable for TLS_ADDITIONAL_ROOT_CA_LIST to prevent security risks from
uncontrolled global state mutation and lack of access control. Refactor the code
to encapsulate the root CA list within a module or class, provide controlled
methods to add or retrieve certificates, and implement validation checks to
ensure only trusted certificates are added, thereby improving security and
reliability.
src/utils/additional-root-cas.js
Outdated
Comment on lines
4
to
185
| TLS_ADDITIONAL_ROOT_CA_LIST.push([ | ||
| `-----BEGIN CERTIFICATE----- | ||
| MIIFjDCCA3SgAwIBAgIQfx8skC6D0OO2+zvuR4tegDANBgkqhkiG9w0BAQsFADBM | ||
| MSAwHgYDVQQLExdHbG9iYWxTaWduIFJvb3QgQ0EgLSBSNjETMBEGA1UEChMKR2xv | ||
| YmFsU2lnbjETMBEGA1UEAxMKR2xvYmFsU2lnbjAeFw0yMzA3MTkwMzQzMjVaFw0y | ||
| NjA3MTkwMDAwMDBaMFUxCzAJBgNVBAYTAkJFMRkwFwYDVQQKExBHbG9iYWxTaWdu | ||
| IG52LXNhMSswKQYDVQQDEyJHbG9iYWxTaWduIEdDQyBSNiBBbHBoYVNTTCBDQSAy | ||
| MDIzMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEA00Jvk5ADppO0rgDn | ||
| j1M14XIb032Aas409JJFAb8cUjipFOth7ySLdaWLe3s63oSs5x3eWwzTpX4BFkzZ | ||
| bxT1eoJSHfT2M0wZ5QOPcCIjsr+YB8TAvV2yJSyq+emRrN/FtgCSTaWXSJ5jipW8 | ||
| SJ/VAuXPMzuAP2yYpuPcjjQ5GyrssDXgu+FhtYxqyFP7BSvx9jQhh5QV5zhLycua | ||
| n8n+J0Uw09WRQK6JGQ5HzDZQinkNel+fZZNRG1gE9Qeh+tHBplrkalB1g85qJkPO | ||
| J7SoEvKsmDkajggk/sSq7NPyzFaa/VBGZiRRG+FkxCBniGD5618PQ4trcwHyMojS | ||
| FObOHQIDAQABo4IBXzCCAVswDgYDVR0PAQH/BAQDAgGGMB0GA1UdJQQWMBQGCCsG | ||
| AQUFBwMBBggrBgEFBQcDAjASBgNVHRMBAf8ECDAGAQH/AgEAMB0GA1UdDgQWBBS9 | ||
| BbfzipM8c8t5+g+FEqF3lhiRdDAfBgNVHSMEGDAWgBSubAWjkxPioufi1xzWx/B/ | ||
| yGdToDB7BggrBgEFBQcBAQRvMG0wLgYIKwYBBQUHMAGGImh0dHA6Ly9vY3NwMi5n | ||
| bG9iYWxzaWduLmNvbS9yb290cjYwOwYIKwYBBQUHMAKGL2h0dHA6Ly9zZWN1cmUu | ||
| Z2xvYmFsc2lnbi5jb20vY2FjZXJ0L3Jvb3QtcjYuY3J0MDYGA1UdHwQvMC0wK6Ap | ||
| oCeGJWh0dHA6Ly9jcmwuZ2xvYmFsc2lnbi5jb20vcm9vdC1yNi5jcmwwIQYDVR0g | ||
| BBowGDAIBgZngQwBAgEwDAYKKwYBBAGgMgoBAzANBgkqhkiG9w0BAQsFAAOCAgEA | ||
| fMkkMo5g4mn1ft4d4xR2kHzYpDukhC1XYPwfSZN3A9nEBadjdKZMH7iuS1vF8uSc | ||
| g26/30DRPen2fFRsr662ECyUCR4OfeiiGNdoQvcesM9Xpew3HLQP4qHg+s774hNL | ||
| vGRD4aKSKwFqLMrcqCw6tEAfX99tFWsD4jzbC6k8tjSLzEl0fTUlfkJaWpvLVkpg | ||
| 9et8tD8d51bymCg5J6J6wcXpmsSGnksBobac1+nXmgB7jQC9edU8Z41FFo87BV3k | ||
| CtrWWsdkQavObMsXUPl/AO8y/jOuAWz0wyvPnKom+o6W4vKDY6/6XPypNdebOJ6m | ||
| jyaILp0quoQvhjx87BzENh5s57AIOyIGpS0sDEChVDPzLEfRsH2FJ8/W5woF0nvs | ||
| BTqfYSCqblQbHeDDtCj7Mlf8JfqaMuqcbE4rMSyfeHyCdZQwnc/r9ujnth691AJh | ||
| xyYeCM04metJIe7cB6d4dFm+Pd5ervY4x32r0uQ1Q0spy1VjNqUJjussYuXNyMmF | ||
| HSuLQQ6PrePmH5lcSMQpYKzPoD/RiNVD/PK0O3vuO5vh3o7oKb1FfzoanDsFFTrw | ||
| 0aLOdRW/tmLPWVNVlAb8ad+B80YJsL4HXYnQG8wYAFb8LhwSDyT9v+C1C1lcIHE7 | ||
| nE0AAp9JSHxDYsma9pi4g0Phg3BgOm2euTRzw7R0SzU= | ||
| -----END CERTIFICATE-----`, // GlobalSign GCC R6 AlphaSSL CA 2023 intermediate till 2026 | ||
| `-----BEGIN CERTIFICATE----- | ||
| MIIGGTCCBAGgAwIBAgIQE31TnKp8MamkM3AZaIR6jTANBgkqhkiG9w0BAQwFADCB | ||
| iDELMAkGA1UEBhMCVVMxEzARBgNVBAgTCk5ldyBKZXJzZXkxFDASBgNVBAcTC0pl | ||
| cnNleSBDaXR5MR4wHAYDVQQKExVUaGUgVVNFUlRSVVNUIE5ldHdvcmsxLjAsBgNV | ||
| BAMTJVVTRVJUcnVzdCBSU0EgQ2VydGlmaWNhdGlvbiBBdXRob3JpdHkwHhcNMTgx | ||
| MTAyMDAwMDAwWhcNMzAxMjMxMjM1OTU5WjCBlTELMAkGA1UEBhMCR0IxGzAZBgNV | ||
| BAgTEkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4GA1UEBxMHU2FsZm9yZDEYMBYGA1UE | ||
| ChMPU2VjdGlnbyBMaW1pdGVkMT0wOwYDVQQDEzRTZWN0aWdvIFJTQSBPcmdhbml6 | ||
| YXRpb24gVmFsaWRhdGlvbiBTZWN1cmUgU2VydmVyIENBMIIBIjANBgkqhkiG9w0B | ||
| AQEFAAOCAQ8AMIIBCgKCAQEAnJMCRkVKUkiS/FeN+S3qU76zLNXYqKXsW2kDwB0Q | ||
| 9lkz3v4HSKjojHpnSvH1jcM3ZtAykffEnQRgxLVK4oOLp64m1F06XvjRFnG7ir1x | ||
| on3IzqJgJLBSoDpFUd54k2xiYPHkVpy3O/c8Vdjf1XoxfDV/ElFw4Sy+BKzL+k/h | ||
| fGVqwECn2XylY4QZ4ffK76q06Fha2ZnjJt+OErK43DOyNtoUHZZYQkBuCyKFHFEi | ||
| rsTIBkVtkuZntxkj5Ng2a4XQf8dS48+wdQHgibSov4o2TqPgbOuEQc6lL0giE5dQ | ||
| YkUeCaXMn2xXcEAG2yDoG9bzk4unMp63RBUJ16/9fAEc2wIDAQABo4IBbjCCAWow | ||
| HwYDVR0jBBgwFoAUU3m/WqorSs9UgOHYm8Cd8rIDZsswHQYDVR0OBBYEFBfZ1iUn | ||
| Z/kxwklD2TA2RIxsqU/rMA4GA1UdDwEB/wQEAwIBhjASBgNVHRMBAf8ECDAGAQH/ | ||
| AgEAMB0GA1UdJQQWMBQGCCsGAQUFBwMBBggrBgEFBQcDAjAbBgNVHSAEFDASMAYG | ||
| BFUdIAAwCAYGZ4EMAQICMFAGA1UdHwRJMEcwRaBDoEGGP2h0dHA6Ly9jcmwudXNl | ||
| cnRydXN0LmNvbS9VU0VSVHJ1c3RSU0FDZXJ0aWZpY2F0aW9uQXV0aG9yaXR5LmNy | ||
| bDB2BggrBgEFBQcBAQRqMGgwPwYIKwYBBQUHMAKGM2h0dHA6Ly9jcnQudXNlcnRy | ||
| dXN0LmNvbS9VU0VSVHJ1c3RSU0FBZGRUcnVzdENBLmNydDAlBggrBgEFBQcwAYYZ | ||
| aHR0cDovL29jc3AudXNlcnRydXN0LmNvbTANBgkqhkiG9w0BAQwFAAOCAgEAThNA | ||
| lsnD5m5bwOO69Bfhrgkfyb/LDCUW8nNTs3Yat6tIBtbNAHwgRUNFbBZaGxNh10m6 | ||
| pAKkrOjOzi3JKnSj3N6uq9BoNviRrzwB93fVC8+Xq+uH5xWo+jBaYXEgscBDxLmP | ||
| bYox6xU2JPti1Qucj+lmveZhUZeTth2HvbC1bP6mESkGYTQxMD0gJ3NR0N6Fg9N3 | ||
| OSBGltqnxloWJ4Wyz04PToxcvr44APhL+XJ71PJ616IphdAEutNCLFGIUi7RPSRn | ||
| R+xVzBv0yjTqJsHe3cQhifa6ezIejpZehEU4z4CqN2mLYBd0FUiRnG3wTqN3yhsc | ||
| SPr5z0noX0+FCuKPkBurcEya67emP7SsXaRfz+bYipaQ908mgWB2XQ8kd5GzKjGf | ||
| FlqyXYwcKapInI5v03hAcNt37N3j0VcFcC3mSZiIBYRiBXBWdoY5TtMibx3+bfEO | ||
| s2LEPMvAhblhHrrhFYBZlAyuBbuMf1a+HNJav5fyakywxnB2sJCNwQs2uRHY1ihc | ||
| 6k/+JLcYCpsM0MF8XPtpvcyiTcaQvKZN8rG61ppnW5YCUtCC+cQKXA0o4D/I+pWV | ||
| idWkvklsQLI+qGu41SWyxP7x09fn1txDAXYw+zuLXfdKiXyaNb78yvBXAfCNP6CH | ||
| MntHWpdLgtJmwsQt6j8k9Kf5qLnjatkYYaA7jBU= | ||
| -----END CERTIFICATE-----`, //Sectigo RSA Organization Validation Secure Server CA | ||
| `-----BEGIN CERTIFICATE----- | ||
| MIII+DCCB+CgAwIBAgIQbAP9+jGpC4MAqlBK9HsanzANBgkqhkiG9w0BAQsFADCB | ||
| lTELMAkGA1UEBhMCR0IxGzAZBgNVBAgTEkdyZWF0ZXIgTWFuY2hlc3RlcjEQMA4G | ||
| A1UEBxMHU2FsZm9yZDEYMBYGA1UEChMPU2VjdGlnbyBMaW1pdGVkMT0wOwYDVQQD | ||
| EzRTZWN0aWdvIFJTQSBPcmdhbml6YXRpb24gVmFsaWRhdGlvbiBTZWN1cmUgU2Vy | ||
| dmVyIENBMB4XDTI0MDgyNjAwMDAwMFoXDTI1MDgyNjIzNTk1OVowXTELMAkGA1UE | ||
| BhMCVVMxEzARBgNVBAgTCkNhbGlmb3JuaWExHDAaBgNVBAoTE1N0YXRlIG9mIENh | ||
| bGlmb3JuaWExGzAZBgNVBAMTEmNvbm5lY3QuZGNhLmNhLmdvdjCCAiIwDQYJKoZI | ||
| hvcNAQEBBQADggIPADCCAgoCggIBALqonPJQIJB5rqVt3lFhVCAWVDaUiHINR/0a | ||
| BtHeqP2Ue5EsLGwdCNCRrj8ge0bCqQh25UBEmIOrTWU3HcmyBYPG51TPp+T5GER2 | ||
| r8daV5oqSFpVIThArZF58Omwsbv38hkNn1LCdZher/yqbuZJNHZd2Z/h3Xv410us | ||
| y2EnrALnoKkRUvJ/hfX3Wpn9H+gYILEjwS3Bz4RZbMNnZCmaKFvKdk4hL/5Nyfgi | ||
| ysHgJIM1jTitd24gilbA9RTLpak7naSxevb0SVa48hywpN8zoeDnOE/QIPGZ3CDJ | ||
| 70zHpZ9/T+soTtnTOAkVR3gCq6ZNshfizV6hqQTIvk6w8Ce7AoHv47EIRDwpb6RD | ||
| gODEyZJFxR27/lZrXq3yvaiE0ZXkBFjJ6B4N+IuxpKrflRuddv5ObOm9AxunUsCM | ||
| bhSf+7M8ECKk9j/IPYoKChfhxOyDQPKZSUtHx94+L5Z+7ri01S5ahkVlIY9O7VRz | ||
| PU3YoqNslBUpAIrRYfjN1ej5FGgo867i5RUB9deFgJ/DMwbT0WN9e5DkVDcREbl4 | ||
| mJDRife0nZW88GgLDgFvvw3aFna+MtvE9BKgnTnPUUB9yiRJryj0i0qkIV30XF62 | ||
| CuNYEn8V24VvRv95wnsT6W758DGY7BspK18XVwL+LiA+GvkMFehhIRW6BBw1Txv9 | ||
| +NYVTTm1AgMBAAGjggR5MIIEdTAfBgNVHSMEGDAWgBQX2dYlJ2f5McJJQ9kwNkSM | ||
| bKlP6zAdBgNVHQ4EFgQUYXACOqlHJoJQcEG3L0ICy9gYsV4wDgYDVR0PAQH/BAQD | ||
| AgWgMAwGA1UdEwEB/wQCMAAwHQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUFBwMC | ||
| MEoGA1UdIARDMEEwNQYMKwYBBAGyMQECAQMEMCUwIwYIKwYBBQUHAgEWF2h0dHBz | ||
| Oi8vc2VjdGlnby5jb20vQ1BTMAgGBmeBDAECAjBaBgNVHR8EUzBRME+gTaBLhklo | ||
| dHRwOi8vY3JsLnNlY3RpZ28uY29tL1NlY3RpZ29SU0FPcmdhbml6YXRpb25WYWxp | ||
| ZGF0aW9uU2VjdXJlU2VydmVyQ0EuY3JsMIGKBggrBgEFBQcBAQR+MHwwVQYIKwYB | ||
| BQUHMAKGSWh0dHA6Ly9jcnQuc2VjdGlnby5jb20vU2VjdGlnb1JTQU9yZ2FuaXph | ||
| dGlvblZhbGlkYXRpb25TZWN1cmVTZXJ2ZXJDQS5jcnQwIwYIKwYBBQUHMAGGF2h0 | ||
| dHA6Ly9vY3NwLnNlY3RpZ28uY29tMIIBfgYKKwYBBAHWeQIEAgSCAW4EggFqAWgA | ||
| dQDd3Mo0ldfhFgXnlTL6x5/4PRxQ39sAOhQSdgosrLvIKgAAAZGPYefYAAAEAwBG | ||
| MEQCIC3PuRJmsoXOdITZPFofbx+GkT9JlXYA4rFD66SEzQYNAiBFdkL0000FzUHJ | ||
| A11IglFWjubgpuCz7ct1NqW7nDwxFQB2AA3h8jAr0w3BQGISCepVLvxHdHyx1+kw | ||
| 7w5CHrR+Tqo0AAABkY9h57UAAAQDAEcwRQIhAKN6rHqqCeFQKpBS36UE+z/kTXru | ||
| A7bI/NrW6k+vZXNBAiBJ1mayfSQJPX3LVpPBNK1kHIXI612M9Tpmrt9prraFlgB3 | ||
| ABLxTjS9U3JMhAYZw48/ehP457Vih4icbTAFhOvlhiY6AAABkY9h57UAAAQDAEgw | ||
| RgIhALJAVx2+PVTOBjKeEkYLyTChpUyITMx2yJoZ8Zxe4C1nAiEAyKSU0BY4Wu/e | ||
| du3YZFHbBymWlfsDCPtkUYUXuaZPqrMwggE9BgNVHREEggE0MIIBMIISY29ubmVj | ||
| dC5kY2EuY2EuZ292ghVjb25uZWN0LXdzLmNhYi5jYS5nb3aCFWNvbm5lY3Qtd3Mu | ||
| Y2ZiLmNhLmdvdoIlY29ubmVjdC13cy5jb3VydHJlcG9ydGVyc2JvYXJkLmNhLmdv | ||
| doIVY29ubmVjdC13cy5kY2EuY2EuZ292ghZjb25uZWN0LXdzLmxhdGMuY2EuZ292 | ||
| ghtjb25uZWN0LXdzLnBlc3Rib2FyZC5jYS5nb3aCEmNvbm5lY3QuY2FiLmNhLmdv | ||
| doISY29ubmVjdC5jZmIuY2EuZ292giJjb25uZWN0LmNvdXJ0cmVwb3J0ZXJzYm9h | ||
| cmQuY2EuZ292ghNjb25uZWN0LmxhdGMuY2EuZ292ghhjb25uZWN0LnBlc3Rib2Fy | ||
| ZC5jYS5nb3YwDQYJKoZIhvcNAQELBQADggEBAH8SsgW//ibqOZhMifgDLy2z4srI | ||
| OwYMaWi0mxRO/6fgCO9BcpvT22vrMZYo3JuaEHtKT0joh5mdsfm/3tttEgnFYV5h | ||
| gK4xgkZ/BbXoKWi+lmZPvxQJJFoRRg1WPnTvH+S7hUS0JAi4Wzmt7GGKhKnr5Fp3 | ||
| qTMIS9g0NQNGrV9pYqK1AQFzk0BBdemBqzUHLQjJ1k176AlvXP7xjW9Fi/Fdasat | ||
| dfOtR3XILf1FTAjKGeGS9q2e4h6aZvLmdsDlCiG+YocUpTIOtdiF00zA4MybExyZ | ||
| pfy9x5+dKWTyekk5jr54LEFQ5kUDJaGZ0KnDuOxhDSpAO/Yb/Z/3ZAk2G0s= | ||
| -----END CERTIFICATE-----`, //connect.dca.ca.gov | ||
| `-----BEGIN CERTIFICATE----- | ||
| MIIEjTCCA3WgAwIBAgIQDQd4KhM/xvmlcpbhMf/ReTANBgkqhkiG9w0BAQsFADBh | ||
| MQswCQYDVQQGEwJVUzEVMBMGA1UEChMMRGlnaUNlcnQgSW5jMRkwFwYDVQQLExB3 | ||
| d3cuZGlnaWNlcnQuY29tMSAwHgYDVQQDExdEaWdpQ2VydCBHbG9iYWwgUm9vdCBH | ||
| MjAeFw0xNzExMDIxMjIzMzdaFw0yNzExMDIxMjIzMzdaMGAxCzAJBgNVBAYTAlVT | ||
| MRUwEwYDVQQKEwxEaWdpQ2VydCBJbmMxGTAXBgNVBAsTEHd3dy5kaWdpY2VydC5j | ||
| b20xHzAdBgNVBAMTFkdlb1RydXN0IFRMUyBSU0EgQ0EgRzEwggEiMA0GCSqGSIb3 | ||
| DQEBAQUAA4IBDwAwggEKAoIBAQC+F+jsvikKy/65LWEx/TMkCDIuWegh1Ngwvm4Q | ||
| yISgP7oU5d79eoySG3vOhC3w/3jEMuipoH1fBtp7m0tTpsYbAhch4XA7rfuD6whU | ||
| gajeErLVxoiWMPkC/DnUvbgi74BJmdBiuGHQSd7LwsuXpTEGG9fYXcbTVN5SATYq | ||
| DfbexbYxTMwVJWoVb6lrBEgM3gBBqiiAiy800xu1Nq07JdCIQkBsNpFtZbIZhsDS | ||
| fzlGWP4wEmBQ3O67c+ZXkFr2DcrXBEtHam80Gp2SNhou2U5U7UesDL/xgLK6/0d7 | ||
| 6TnEVMSUVJkZ8VeZr+IUIlvoLrtjLbqugb0T3OYXW+CQU0kBAgMBAAGjggFAMIIB | ||
| PDAdBgNVHQ4EFgQUlE/UXYvkpOKmgP792PkA76O+AlcwHwYDVR0jBBgwFoAUTiJU | ||
| IBiV5uNu5g/6+rkS7QYXjzkwDgYDVR0PAQH/BAQDAgGGMB0GA1UdJQQWMBQGCCsG | ||
| AQUFBwMBBggrBgEFBQcDAjASBgNVHRMBAf8ECDAGAQH/AgEAMDQGCCsGAQUFBwEB | ||
| BCgwJjAkBggrBgEFBQcwAYYYaHR0cDovL29jc3AuZGlnaWNlcnQuY29tMEIGA1Ud | ||
| HwQ7MDkwN6A1oDOGMWh0dHA6Ly9jcmwzLmRpZ2ljZXJ0LmNvbS9EaWdpQ2VydEds | ||
| b2JhbFJvb3RHMi5jcmwwPQYDVR0gBDYwNDAyBgRVHSAAMCowKAYIKwYBBQUHAgEW | ||
| HGh0dHBzOi8vd3d3LmRpZ2ljZXJ0LmNvbS9DUFMwDQYJKoZIhvcNAQELBQADggEB | ||
| AIIcBDqC6cWpyGUSXAjjAcYwsK4iiGF7KweG97i1RJz1kwZhRoo6orU1JtBYnjzB | ||
| c4+/sXmnHJk3mlPyL1xuIAt9sMeC7+vreRIF5wFBC0MCN5sbHwhNN1JzKbifNeP5 | ||
| ozpZdQFmkCo+neBiKR6HqIA+LMTMCMMuv2khGGuPHmtDze4GmEGZtYLyF8EQpa5Y | ||
| jPuV6k2Cr/N3XxFpT3hRpt/3usU/Zb9wfKPtWpoznZ4/44c1p9rzFcZYrWkj3A+7 | ||
| TNBJE0GmP2fhXhP1D/XVfIW/h0yCJGEiV9Glm/uGOa3DXHlmbAcxSyCRraG+ZBkA | ||
| 7h4SeM6Y8l/7MBRpPCz6l8Y= | ||
| -----END CERTIFICATE-----`, //GeoTrust TLS RSA CA G1 | ||
| `-----BEGIN CERTIFICATE----- | ||
| MIIGTDCCBDSgAwIBAgIQOXpmzCdWNi4NqofKbqvjsTANBgkqhkiG9w0BAQwFADBf | ||
| MQswCQYDVQQGEwJHQjEYMBYGA1UEChMPU2VjdGlnbyBMaW1pdGVkMTYwNAYDVQQD | ||
| Ey1TZWN0aWdvIFB1YmxpYyBTZXJ2ZXIgQXV0aGVudGljYXRpb24gUm9vdCBSNDYw | ||
| HhcNMjEwMzIyMDAwMDAwWhcNMzYwMzIxMjM1OTU5WjBgMQswCQYDVQQGEwJHQjEY | ||
| MBYGA1UEChMPU2VjdGlnbyBMaW1pdGVkMTcwNQYDVQQDEy5TZWN0aWdvIFB1Ymxp | ||
| YyBTZXJ2ZXIgQXV0aGVudGljYXRpb24gQ0EgRFYgUjM2MIIBojANBgkqhkiG9w0B | ||
| AQEFAAOCAY8AMIIBigKCAYEAljZf2HIz7+SPUPQCQObZYcrxLTHYdf1ZtMRe7Yeq | ||
| RPSwygz16qJ9cAWtWNTcuICc++p8Dct7zNGxCpqmEtqifO7NvuB5dEVexXn9RFFH | ||
| 12Hm+NtPRQgXIFjx6MSJcNWuVO3XGE57L1mHlcQYj+g4hny90aFh2SCZCDEVkAja | ||
| EMMfYPKuCjHuuF+bzHFb/9gV8P9+ekcHENF2nR1efGWSKwnfG5RawlkaQDpRtZTm | ||
| M64TIsv/r7cyFO4nSjs1jLdXYdz5q3a4L0NoabZfbdxVb+CUEHfB0bpulZQtH1Rv | ||
| 38e/lIdP7OTTIlZh6OYL6NhxP8So0/sht/4J9mqIGxRFc0/pC8suja+wcIUna0HB | ||
| pXKfXTKpzgis+zmXDL06ASJf5E4A2/m+Hp6b84sfPAwQ766rI65mh50S0Di9E3Pn | ||
| 2WcaJc+PILsBmYpgtmgWTR9eV9otfKRUBfzHUHcVgarub/XluEpRlTtZudU5xbFN | ||
| xx/DgMrXLUAPaI60fZ6wA+PTAgMBAAGjggGBMIIBfTAfBgNVHSMEGDAWgBRWc1hk | ||
| lfmSGrASKgRieaFAFYghSTAdBgNVHQ4EFgQUaMASFhgOr872h6YyV6NGUV3LBycw | ||
| DgYDVR0PAQH/BAQDAgGGMBIGA1UdEwEB/wQIMAYBAf8CAQAwHQYDVR0lBBYwFAYI | ||
| KwYBBQUHAwEGCCsGAQUFBwMCMBsGA1UdIAQUMBIwBgYEVR0gADAIBgZngQwBAgEw | ||
| VAYDVR0fBE0wSzBJoEegRYZDaHR0cDovL2NybC5zZWN0aWdvLmNvbS9TZWN0aWdv | ||
| UHVibGljU2VydmVyQXV0aGVudGljYXRpb25Sb290UjQ2LmNybDCBhAYIKwYBBQUH | ||
| AQEEeDB2ME8GCCsGAQUFBzAChkNodHRwOi8vY3J0LnNlY3RpZ28uY29tL1NlY3Rp | ||
| Z29QdWJsaWNTZXJ2ZXJBdXRoZW50aWNhdGlvblJvb3RSNDYucDdjMCMGCCsGAQUF | ||
| BzABhhdodHRwOi8vb2NzcC5zZWN0aWdvLmNvbTANBgkqhkiG9w0BAQwFAAOCAgEA | ||
| YtOC9Fy+TqECFw40IospI92kLGgoSZGPOSQXMBqmsGWZUQ7rux7cj1du6d9rD6C8 | ||
| ze1B2eQjkrGkIL/OF1s7vSmgYVafsRoZd/IHUrkoQvX8FZwUsmPu7amgBfaY3g+d | ||
| q1x0jNGKb6I6Bzdl6LgMD9qxp+3i7GQOnd9J8LFSietY6Z4jUBzVoOoz8iAU84OF | ||
| h2HhAuiPw1ai0VnY38RTI+8kepGWVfGxfBWzwH9uIjeooIeaosVFvE8cmYUB4TSH | ||
| 5dUyD0jHct2+8ceKEtIoFU/FfHq/mDaVnvcDCZXtIgitdMFQdMZaVehmObyhRdDD | ||
| 4NQCs0gaI9AAgFj4L9QtkARzhQLNyRf87Kln+YU0lgCGr9HLg3rGO8q+Y4ppLsOd | ||
| unQZ6ZxPNGIfOApbPVf5hCe58EZwiWdHIMn9lPP6+F404y8NNugbQixBber+x536 | ||
| WrZhFZLjEkhp7fFXf9r32rNPfb74X/U90Bdy4lzp3+X1ukh1BuMxA/EEhDoTOS3l | ||
| 7ABvc7BYSQubQ2490OcdkIzUh3ZwDrakMVrbaTxUM2p24N6dB+ns2zptWCva6jzW | ||
| r8IWKIMxzxLPv5Kt3ePKcUdvkBU/smqujSczTzzSjIoR5QqQA6lN1ZRSnuHIWCvh | ||
| JEltkYnTAH41QJ6SAWO66GrrUESwN/cgZzL4JLEqz1Y= | ||
| -----END CERTIFICATE-----` // Sectigo Public Server Authentication CA DV R36 | ||
| ]) No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Implement certificate validation and consider certificate expiry monitoring.
The hard-coded certificates include several with specific expiry dates (e.g., GlobalSign GCC R6 expires in 2026, connect.dca.ca.gov expires in 2025). This approach has several concerns:
- No certificate validation: Certificates are added without verification of their validity or integrity
- Expiry management: No mechanism to track or warn about expiring certificates
- Bundle size: Large PEM strings increase the bundle size significantly
Consider implementing:
- Certificate validation before addition
- Expiry date monitoring with warnings
- External certificate loading mechanism
- Proper error handling for malformed certificates
+/**
+ * Validates and adds certificates to the global root CA list
+ * @param {string[]} certificates - Array of PEM-encoded certificates
+ */
+function addRootCAs(certificates) {
+ const validCertificates = certificates.filter(cert => {
+ // Add certificate validation logic here
+ return cert.includes('-----BEGIN CERTIFICATE-----') &&
+ cert.includes('-----END CERTIFICATE-----')
+ })
+
+ global.TLS_ADDITIONAL_ROOT_CA_LIST.push(...validCertificates)
+}
-TLS_ADDITIONAL_ROOT_CA_LIST.push([
+addRootCAs([
// ... certificates
-])
+])🤖 Prompt for AI Agents
In src/utils/additional-root-cas.js around lines 4 to 185, the current
implementation hard-codes multiple PEM certificates without validating their
integrity or monitoring their expiry dates, which risks using invalid or expired
certificates and increases bundle size. To fix this, implement a certificate
validation step before adding each certificate to ensure they are well-formed
and trusted, add logic to parse and monitor expiry dates to warn about
soon-to-expire certificates, refactor to load certificates from external sources
or configuration files to reduce bundle size, and include error handling to
gracefully manage malformed or invalid certificates.
Comment on lines
+263
to
274
| function loadRootCAs() { | ||
| if(ROOT_CAS) { | ||
| return ROOT_CAS | ||
| } | ||
|
|
||
| ROOT_CAS = MOZILLA_ROOT_CA_LIST.map(loadX509FromPem) | ||
| if(typeof TLS_ADDITIONAL_ROOT_CA_LIST !== 'undefined') { | ||
| ROOT_CAS.push(...TLS_ADDITIONAL_ROOT_CA_LIST.map(loadX509FromPem)) | ||
| } | ||
|
|
||
| return ROOT_CAS | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify global variable type safety and consider error handling.
The lazy loading implementation is well-structured with proper caching. However, consider these aspects:
- Type Safety: The global
TLS_ADDITIONAL_ROOT_CA_LISTlacks type checking beyond existence - Error Handling: Certificate parsing errors from
loadX509FromPemwill propagate uncaught
Verify the global variable declaration and error handling approach:
🏁 Script executed:
#!/bin/bash
# Check if TLS_ADDITIONAL_ROOT_CA_LIST is properly typed in the global declarations
ast-grep --pattern 'declare global {
$$$
TLS_ADDITIONAL_ROOT_CA_LIST: $TYPE
$$$
}'
# Check for any error handling patterns around loadX509FromPem usage
rg -A 3 -B 3 'loadX509FromPem'Length of output: 3684
🏁 Script executed:
#!/bin/bash
# Inspect the implementation of loadX509FromPem to confirm its error handling
head -n 50 src/utils/x509.tsLength of output: 1472
Ensure TLS_ADDITIONAL_ROOT_CA_LIST Is Typed & Parsing Errors Are Handled
The lazy‐loader works, but we’ve uncovered two gaps:
-
Missing Type Declaration
NoTLS_ADDITIONAL_ROOT_CA_LISTdeclaration was found in the repo. Without it, TS treats it asany, losing safety. -
Uncaught Parsing Errors
loadX509FromPemthrows on invalid PEM, andROOT_CAS = …map(loadX509FromPem)will abort the entire load if any entry is bad.
———
Recommended fixes:
-
Declare the global variable (e.g. in
src/types/global.d.ts):declare global { /** Additional root CAs (PEM strings) */ const TLS_ADDITIONAL_ROOT_CA_LIST?: string[]; } export {};
-
Tighten the runtime check in
loadRootCAs()and handle PEM errors:function loadRootCAs() { if (ROOT_CAS) return ROOT_CAS; const loaded: X509Certificate[] = []; for (const pem of MOZILLA_ROOT_CA_LIST) { loaded.push(loadX509FromPem(pem)); // keep fail-fast for Mozilla CAs } if (Array.isArray(TLS_ADDITIONAL_ROOT_CA_LIST)) { for (const pem of TLS_ADDITIONAL_ROOT_CA_LIST) { try { loaded.push(loadX509FromPem(pem)); } catch (err) { console.warn('Skipping invalid additional root CA:', err); } } } ROOT_CAS = loaded; return ROOT_CAS; }
- This preserves fast failure on the official list but skips/logs any bad entries in the optional list.
- Adjust as needed if you prefer an all-or-nothing strategy.
🤖 Prompt for AI Agents
In src/utils/parse-certificate.ts around lines 263 to 274, the global variable
TLS_ADDITIONAL_ROOT_CA_LIST is missing a TypeScript declaration, causing it to
be typed as any and losing type safety. Add a declaration for it in
src/types/global.d.ts as an optional string array. Also, modify loadRootCAs() to
check if TLS_ADDITIONAL_ROOT_CA_LIST is an array before processing, and wrap
each loadX509FromPem call for these additional CAs in a try-catch block to
handle and log parsing errors without aborting the entire load, while keeping
fail-fast behavior for the MOZILLA_ROOT_CA_LIST entries.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Testing (ignore for documentation update)
Type of change
Checklist:
Additional Notes:
Summary by CodeRabbit
New Features
Refactor
Chores