Fix HTTPS for non-Let's Encrypt certificates

[zmknox]

In our use of this library, my team and I have used a custom certificate (provided by the cert and key options in the ssl object) to handle HTTPS. We don't need Let's Encrypt as this is an internal use case. As I was updating our project to use the 1.x release of Redbird, I was facing issues connecting to the proxy over HTTPS.

When I went digging, I found that the SNICallback function set up within setupHttpsProxy was making some assumptions that everybody was using Let's Encrypt. I was able to resolve this by adding a check for the letsencrypt option (and removing the extra else condition which was also causing issues).

I don't think this should affect anything about Let's Encrypt use cases, but it allows my internal project using custom certificates to work again using the 1.x release of Redbird.

(The workflow action version update in this PR was simply because the job was failing for my commits)

[manast]

@copilot please add some test cases to cover this new functionality.

[Copilot]

Pull request overview

Fixes HTTPS proxy behavior for deployments using custom (non-Let’s Encrypt) certificates by reducing Let’s Encrypt-specific assumptions in SNI handling, and updates CI workflow configuration.

Changes:

• Gate Let’s Encrypt certificate-loading logic in SNICallback behind opts.letsencrypt being configured.
• Remove a redundant/incorrect SNI error path that prevented non-Let’s Encrypt HTTPS usage.
• Update GitHub Actions test matrix Node versions and bump referenced Action versions.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
lib/proxy.ts	Adjusts SNI callback logic so custom/default cert setups work without Let’s Encrypt.
.github/workflows/test.yml	Updates Node test matrix and bumps checkout/setup-node action versions.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The !certs[hostname] check treats certs[host] = void 0 (used elsewhere to explicitly mean “use default certificates”) the same as “no entry for this hostname”. If this.opts.letsencrypt is enabled globally, hostnames intentionally configured to use the default cert will still enter the Let’s Encrypt loading path and may error. Consider distinguishing “missing” vs “present but undefined” (e.g., !(hostname in certs) / !Object.hasOwn(certs, hostname)) before attempting Let’s Encrypt.

Suggested change

	if (this.opts?.letsencrypt && !certs[hostname]) {
	// Only attempt to load Let's Encrypt certificates when there is no
	// explicit entry for this hostname in the certs map.
	if (this.opts?.letsencrypt && !Object.prototype.hasOwnProperty.call(certs, hostname)) {

[zmknox]

This might be useful, but my intention with my PR was to be a pretty minimal fix. So I kept the existing conditional and added the single check for letsencrypt

[Copilot]

This change affects HTTPS behavior when letsencrypt is not configured (custom/default certificates via ssl.key/ssl.cert). There are existing Vitest specs covering Let’s Encrypt certificate generation, but no spec that exercises an actual HTTPS request using a non-Let’s Encrypt custom certificate to prevent regressions of this path. Adding an integration-style test similar to the existing HTTPS request tests would lock in the fix.

[zmknox]

When a custom certificate is provided, I don't believe that checking for certs in this way is useful or relevant. I have tested this code in my team's own project and it only worked when it simply used the cert I provided without interference here.

[manast]

@copilot add relevant tests to cover this new functionality.

[zmknox]

@manast I added a few tests myself since Copilot didn't seem to listen to your request.

(In these, I also replaced the test cert that was there prior. I think the expiration of October 2024 was causing an issue with the new tests I was trying to make, so I generated one with a 10 year lifespan)