fix: use caFile variable instead of hardcoded "cacert" in configureTLS#388
Merged
tomasmik merged 3 commits intospacelift-io:mainfrom Apr 20, 2026
Merged
Conversation
The SPACELIFT_API_TLS_CA env var was read correctly but os.ReadFile was called with the literal string "cacert" instead of the caFile variable, making the feature completely non-functional. Also adds SSL_CERT_FILE as a fallback env var, which is the standard way Go programs accept custom CA bundles. This is especially useful in sandboxed macOS environments where Security.framework cannot access the Keychain (OSStatus -26276). Setting tls.Config.RootCAs to a non-nil pool forces Go's pure-Go verifier, bypassing the Keychain dependency.
Covers SPACELIFT_API_TLS_CA, SSL_CERT_FILE fallback, precedence order, invalid path, and invalid certificate content.
Contributor
|
@bernardm-k can you fix the tests? |
The previous writeTempCA() used a hand-crafted fake PEM that isn't valid X.509. On macOS it worked because /etc/ssl/cert.pem was used as a fallback, but on Ubuntu CI (where that file doesn't exist) the fake cert failed to parse. Generate a real ECDSA P-256 self-signed CA certificate at test time using Go's crypto stdlib, which works on all platforms.
Contributor
Author
|
I believe it is fixed now. |
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
2 participants
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.
Summary
configureTLS()readsSPACELIFT_API_TLS_CAintocaFilebut passes the hardcoded string"cacert"toos.ReadFile, making the CA bundle feature non-functional. This also addsSSL_CERT_FILEas a fallback.Changes
internal/cmd/authenticated/client.goProblem: Line 84 calls
os.ReadFile("cacert")instead ofos.ReadFile(caFile). SettingSPACELIFT_API_TLS_CA=/etc/ssl/cert.pemtries to read a literal file namedcacertin the current directory.Solution:
os.ReadFileto use thecaFilevariableSSL_CERT_FILEenv var as a fallback whenSPACELIFT_API_TLS_CAis not setSPACELIFT_API_TLS_CAtakes precedence when both are setImpact: Unblocks spacectl usage in sandboxed macOS environments (e.g., restricted CI runners, containerized dev environments) where Security.framework cannot access the Keychain, causing
x509: OSStatus -26276. Settingtls.Config.RootCAsto a non-nil pool forces Go's pure-Go certificate verifier, bypassing the Security.framework dependency entirely.internal/cmd/authenticated/client_test.go(new)Adds 6 test cases for
configureTLS:SPACELIFT_API_TLS_CAset → RootCAs populatedSSL_CERT_FILEfallback → RootCAs populatedSPACELIFT_API_TLS_CAtakes precedenceerrEnvSpaceliftAPIClientCAParseNet Impact
SPACELIFT_API_TLS_CAhas never workedSSL_CERT_FILEsupport used by other Go toolingTest plan
go test ./internal/cmd/authenticated/...passesSPACELIFT_API_TLS_CA=/etc/ssl/cert.pemand verifyspacectl profile listworks in a sandboxed environment