Repository navigation
fix: trust the certificates named by SSL_CERT_FILE on Windows and macOS - #141
Merged
Merged
Conversation
Go reads that variable only on Unix; elsewhere it verifies through the operating system. The legacy CLI reads it on every platform, through Composer\CaBundle, so the two parts of the CLI disagreed about which certificates to trust, and a command implemented in Go failed where the same command implemented in PHP worked. An unusable file is reported as a warning rather than being fatal, and the system certificates are used, as before. Written by Claude Code.
Contributor
|
📋 PR Summary Adds an Changes
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aligns TLS certificate trust behavior between the Go and legacy PHP halves of the Upsun CLI by honoring SSL_CERT_FILE on Windows and macOS (where Go does not read it automatically), so commands implemented in Go (e.g., init) behave consistently with legacy commands.
Changes:
- Add
internal/certswithUseEnvCertFile()to configure the default HTTP transport to trust the PEM bundle referenced bySSL_CERT_FILEon Windows/macOS. - Invoke
certs.UseEnvCertFile()early incmd/platform/main.go, warning (non-fatal) if the file is unreadable or invalid. - Add tests in
internal/certscovering trust/no-trust behavior and error cases for invalid/missing PEM input.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| internal/certs/certs.go | Introduces transport configuration to use SSL_CERT_FILE roots on platforms where Go doesn’t read it. |
| internal/certs/certs_test.go | Adds unit tests validating trust behavior and error handling for the cert-file logic. |
| cmd/platform/main.go | Wires the cert-file logic into CLI startup so Go HTTP requests follow the same trust model as legacy PHP. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Written by Claude Code.
bojanz
self-requested a review
August 7, 2026 16:45
bojanz
approved these changes
Aug 7, 2026
![upsun-dispatch[bot]](https://avatars.githubusercontent.com/in/4153575?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Note
Reviewed — No new blocking findings · 1 minor point
🔍 Full review · 3 files reviewed
🔵 Minor points
Not blocking, and no threads opened for these.
internal/certs/certs.go:35— UseEnvCertFile honours only SSL_CERT_FILE. Composer\CaBundle (the legacy CLI's resolver) and Go on Unix both also consult SSL_CERT_DIR. On Windows/macOS a user whose organization sets only SSL_CERT_DIR still gets the legacy-vs-Go divergence that #110 describes: the legacy half trusts the directory bundle while the Go half falls back to the system store, so environment:list works and init fails on the same machine.
Review details
- Commit: 5c0219e
- Model: claude-opus-4-8
- Panel: security · correctness · robustness · design
pjcdawkins
added a commit
that referenced
this pull request
Oct 21, 2026
* fix(legacy): use SSL_CERT_FILE on Windows, as Go does Since #259, the embedded PHP on Windows verifies certificates with Schannel against the Windows certificate store, and ignored SSL_CERT_FILE. Go (since 1.27) still uses it on Windows, so with it set, Go and PHP commands would trust different certificates, as before #141. The wrapper now passes SSL_CERT_FILE to PHP as openssl.cafile, which PHP gives to every curl handle as CURLOPT_CAINFO. Schannel then verifies against that file instead of the store, as v5.15.0 did (through Composer\CaBundle) and as Go does. Without SSL_CERT_FILE, nothing is passed and the store is used. An unusable file makes requests fail, as in Go. SSL_CERT_DIR is still not supported on Windows, as Schannel cannot use a directory. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(legacy): let PHP read SSL_CERT_FILE from the environment The wrapper now sets openssl.cafile to "${SSL_CERT_FILE}", which PHP expands itself, instead of quoting and escaping the path. A path with a literal "${...}" was otherwise expanded by PHP's ini parser. The ini escaping helper is removed, and the test checks paths with "~", backslashes, "$" and quotes. Co-Authored-By: Claude Opus 5.5 <[email protected]> * fix(legacy): verify an HTTPS proxy with SSL_CERT_FILE on Windows too PHP passes openssl.cafile to curl as CURLOPT_CAINFO, which only covers servers: curl verifies an HTTPS proxy with CURLOPT_PROXY_CAINFO, without falling back to CAINFO. So with SSL_CERT_FILE set, Schannel still verified a proxy against the Windows store, while Go uses the file for both. With Schannel, getGuzzleOptions() now also sets CURLOPT_PROXY_CAINFO from openssl.cafile when it is set. The Windows test checks both cases through an HTTPS proxy. Co-Authored-By: Claude Opus 5.5 <[email protected]> --------- Co-authored-by: Claude Opus 5.5 <[email protected]>
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
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.
Related to #110. Independent of #139 and #140, which deal with the Windows certificate store.
The problem
The legacy CLI trusts the certificates named by
SSL_CERT_FILEon every platform, becauseComposer\CaBundlechecks that variable first when it resolves a bundle. The Go part of the CLI only trusts it on Unix: Go readsSSL_CERT_FILEincrypto/x509/root_unix.go, and on Windows and macOS it verifies through the operating system instead.So for someone whose organization gives them a CA bundle as a file,
upsun environment:listworks andupsun initdoes not, on the same machine, for the same reason — with no hint that the two halves of the CLI resolve certificates differently.The change
certs.UseEnvCertFile()points the default HTTP transport at the file, on the platforms where Go does not read the variable itself. Every request the Go part makes goes throughhttp.DefaultTransport— the API client, the update check, the config fetch, andinit— so one call at startup covers all of them.The file replaces the system certificates rather than adding to them. That matches Go's own behavior on Unix and
Composer\CaBundle's everywhere, so the CLI resolves certificates the same way whichever part serves the command.An unusable file is a warning on stderr rather than a fatal error, and the system certificates are used, which is what both Go and
Composer\CaBundledo with one.Tests
internal/certsserves HTTPS with a certificate the system does not trust, and checks it is rejected with no file set and accepted with the file set. The error cases cover an unreadable path and a file holding no certificates.Written by Claude Code.