-
Notifications
You must be signed in to change notification settings - Fork 4
[FIX] Added GeoTrust TLS RSA CA G1 #3
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.
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
Conversation
WalkthroughThe changes update the version in Changes
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! 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
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 (
|
|
Alex E seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/utils/root-ca.ts (1)
4450-4454: Consider guarding against future duplicates
ROOT_CA_LIST.push(...ADDITIONAL_ROOT_CA_LIST)appends blindly; with the ever-growing list we now hit duplicates (see lines 118-146).
A one-liner like below eliminates this class of problems and trims memory:-ROOT_CA_LIST.push(...ADDITIONAL_ROOT_CA_LIST) +ROOT_CA_LIST.push( + ...ADDITIONAL_ROOT_CA_LIST.filter(pem => !ROOT_CA_LIST.includes(pem)) +)
🧹 Nitpick comments (1)
package.json (1)
3-3: Version bump looks good, but lock-file & changelog are out of sync
"version": "0.0.3"is updated here, yet there’s no accompanyingpackage-lock.json/pnpm-lock.yamlchange or CHANGELOG entry. CI or consumers installing from the registry may receive mismatched metadata.Action items:
- Commit the regenerated lock-file (
npm install --package-lock-only/pnpm install) so dependency hashes stay consistent.- Add a brief entry to
CHANGELOG.md(e.g., “0.0.3 – Added GeoTrust TLS RSA CA G1 root certificate”).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.json(1 hunks)src/utils/root-ca.ts(4 hunks)
🔇 Additional comments (1)
src/utils/root-ca.ts (1)
1059-1090: Good to see the new D-TRUST 2023 roots addedBoth “BR Root CA 2 2023” and “EV Root CA 2 2023” look correct and are annotated clearly.
No issues spotted here.Also applies to: 1110-1141
| pfy9x5+dKWTyekk5jr54LEFQ5kUDJaGZ0KnDuOxhDSpAO/Yb/Z/3ZAk2G0s= | ||
| -----END CERTIFICATE-----` //Sectigo RSA Organization Validation Secure Server CA | ||
| -----END CERTIFICATE-----`, //Sectigo RSA Organization Validation Secure Server CA | ||
| `-----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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accidental duplicate certificate block
This PEM for “Sectigo RSA Organization Validation Secure Server CA” is already present earlier in the list (lines 70-119).
Leaving both entries bloats the store and forces the X509 parser to work on identical material twice.
- `-----BEGIN CERTIFICATE-----
- ...
- -----END CERTIFICATE-----`, //Sectigo RSA Organization Validation Secure Server CADrop the second copy or, better, deduplicate programmatically before pushing into ROOT_CA_LIST.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/utils/root-ca.ts between lines 118 and 146, there is a duplicate PEM
certificate block for "Sectigo RSA Organization Validation Secure Server CA"
that already appears earlier around lines 70 to 119. To fix this, remove the
second occurrence of this certificate block to avoid redundancy and reduce
processing overhead. Alternatively, implement a deduplication step in the code
that constructs ROOT_CA_LIST to ensure no duplicate certificates are included
before pushing them into the list.
Sajjad21990
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary by CodeRabbit