Skip to content

Conversation

@shruti0085
Copy link
Contributor

@shruti0085 shruti0085 commented Jul 11, 2025

Description of changes:
This change improves the proxy support story for the extension. With this, we honor customer CA cert if specified in the preferences UI. If it is not supplied, instead of leaving it blank, we now detect system certificates and send it over to the node based language server. This allows us to address some issues where users are on corporate proxies/firewalls that have a proxy url but not an explicitly defined cert and expects applications to honor system certs.
We currently do the same system cert detection when downloading artifacts for lsp.

Follows a similar approach as JB: aws/aws-toolkit-jetbrains#5553

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

}

try {
String pemContent = ProxyUtil.getCertificatesAsPem();
Copy link

Choose a reason for hiding this comment

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

this is probably fine, but since eclipse does not really have dynamic certificate mutation it is probably safe to make one copy for the IDE session

if (StringUtils.isEmpty(pemContent)) {
return null;
}
Activator.getLogger().info("Injecting IDE trusted certificates into NODE_EXTRA_CA_CERTS");
Copy link

Choose a reason for hiding this comment

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

print location too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

pemEntries.add(encodedCert);
pemEntries.add("-----END CERTIFICATE-----");
} catch (Exception e) {
Activator.getLogger().error("Failed to encode certificate", e);
Copy link

Choose a reason for hiding this comment

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

maybe skip the cert instead of giving up

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This already does that since the try-catch is within the for loop

@shruti0085 shruti0085 merged commit bd51f6c into main Jul 16, 2025
1 check passed
@shruti0085 shruti0085 deleted the shruti0085/injectCertsToLsp branch July 16, 2025 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants