Skip to content

Commit e2d6ad4

Browse files
committed
Fix systemcheck reporting all checks passed when CA is not trusted
checkCA only verified CA file existence, not system trust. After a new install where the user skips CA installation, first-run said "Setup incomplete" but systemcheck immediately said "All checks passed".
1 parent 6820cc4 commit e2d6ad4

4 files changed

Lines changed: 44 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
1+
## v0.5.3
2+
3+
### Bug Fixes
4+
5+
- **Fix systemcheck reporting "All checks passed" when CA is not trusted** - `checkCA` only checked whether CA files exist, not whether the CA is trusted by the system. On a new install where the user skips CA installation, first-run would say "Setup incomplete" but systemcheck would say "All checks passed!" immediately after. Now systemcheck verifies trust status and reports "not trusted by system" with instructions to fix.
6+
17
## v0.5.2
28

39
### Bug Fixes

cmd/systemcheck.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func runSystemcheck(cmd *cobra.Command, args []string) error {
195195

196196
// Check local CA (only if mkcert is available)
197197
if mkcertPath != "" {
198-
issues += checkCA(ctx, mkcertPath)
198+
issues += checkCA(ctx, mkcertPath, globalCfg)
199199
}
200200

201201
// Check certificates
@@ -291,7 +291,7 @@ func checkMkcert(ctx context.Context, cfg *config.GlobalConfig) (string, int) {
291291
return "", 1
292292
}
293293

294-
func checkCA(ctx context.Context, mkcertPath string) int {
294+
func checkCA(ctx context.Context, mkcertPath string, cfg *config.GlobalConfig) int {
295295
fmt.Print("Local CA: ")
296296

297297
mkcert := tools.NewMkcert(mkcertPath)
@@ -314,6 +314,17 @@ func checkCA(ctx context.Context, mkcertPath string) int {
314314
return 1
315315
}
316316

317+
// CA files exist - also check if trusted by the system
318+
certPath := filepath.Join(config.GetCertsDir(), ssl.CertFileName)
319+
if _, err := os.Stat(certPath); err == nil {
320+
trusted, err := mkcert.IsCATrusted(ctx, certPath)
321+
if err == nil && !trusted {
322+
fmt.Printf("%s (%s - not trusted by system)\n", statusText("MISSING"), caRoot)
323+
fmt.Println(" Run 'scdev systemcheck --install-ca' to install")
324+
return 1
325+
}
326+
}
327+
317328
fmt.Printf("%s (%s)\n", statusText("OK"), caRoot)
318329
return 0
319330
}

internal/firstrun/firstrun.go

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import (
55
"fmt"
66
"os"
77
"path/filepath"
8-
"runtime"
98

109
"github.com/ScaleCommerce-DEV/scdev/internal/config"
1110
"github.com/ScaleCommerce-DEV/scdev/internal/ssl"
@@ -126,7 +125,7 @@ func (m *Manager) RunSetup(ctx context.Context) (bool, error) {
126125
// Step 4: Verify CA is trusted (by checking if cert is valid)
127126
fmt.Println("[4/4] Verifying CA trust...")
128127
if certPath != "" {
129-
trusted, err := m.verifyCertTrust(ctx, certPath)
128+
trusted, err := m.verifyCertTrust(ctx, mkcertPath, certPath)
130129
if err != nil {
131130
fmt.Printf(" Warning: could not verify trust: %v\n", err)
132131
} else if trusted {
@@ -169,7 +168,7 @@ func (m *Manager) RunSetup(ctx context.Context) (bool, error) {
169168
}
170169

171170
// Re-verify that CA is now trusted
172-
trustedNow, err := m.verifyCertTrust(ctx, certPath)
171+
trustedNow, err := m.verifyCertTrust(ctx, mkcertPath, certPath)
173172
if err != nil || !trustedNow {
174173
fmt.Println(" CA installation completed but verification still fails.")
175174
fmt.Println(" You may need to restart your browser or system.")
@@ -266,23 +265,7 @@ func (m *Manager) ensureCertsWithPath(ctx context.Context, mkcertPath string) (b
266265

267266
// verifyCertTrust checks if the certificate is trusted by the system
268267
// Returns (trusted, error)
269-
func (m *Manager) verifyCertTrust(ctx context.Context, certPath string) (bool, error) {
270-
if runtime.GOOS == "darwin" {
271-
// On macOS, use security verify-cert
272-
_, err := tools.RunTool(ctx, "security", "verify-cert", "-c", certPath)
273-
if err != nil {
274-
// Verification failed - CA not trusted
275-
return false, nil
276-
}
277-
return true, nil
278-
}
279-
280-
// On Linux, use openssl verify against system trust store
281-
// This will fail if the mkcert CA is not installed in the system
282-
_, err := tools.RunTool(ctx, "openssl", "verify", certPath)
283-
if err != nil {
284-
// Verification failed - CA not in system trust store
285-
return false, nil
286-
}
287-
return true, nil
268+
func (m *Manager) verifyCertTrust(ctx context.Context, mkcertPath, certPath string) (bool, error) {
269+
mkcert := tools.NewMkcert(mkcertPath)
270+
return mkcert.IsCATrusted(ctx, certPath)
288271
}

internal/tools/mkcert.go

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"fmt"
66
"os"
77
"path/filepath"
8+
"runtime"
89

910
"github.com/ScaleCommerce-DEV/scdev/internal/config"
1011
)
@@ -101,6 +102,25 @@ func (m *Mkcert) GenerateCert(ctx context.Context, certPath, keyPath string, dom
101102
return nil
102103
}
103104

105+
// IsCATrusted checks if the mkcert CA is trusted by the system.
106+
// It does this by verifying the certificate at certPath against the system trust store.
107+
func (m *Mkcert) IsCATrusted(ctx context.Context, certPath string) (bool, error) {
108+
if runtime.GOOS == "darwin" {
109+
_, err := RunTool(ctx, "security", "verify-cert", "-c", certPath)
110+
if err != nil {
111+
return false, nil
112+
}
113+
return true, nil
114+
}
115+
116+
// On Linux, use openssl verify against system trust store
117+
_, err := RunTool(ctx, "openssl", "verify", certPath)
118+
if err != nil {
119+
return false, nil
120+
}
121+
return true, nil
122+
}
123+
104124
// Version returns the mkcert version
105125
func (m *Mkcert) Version(ctx context.Context) (string, error) {
106126
output, err := RunTool(ctx, m.binaryPath, "-version")

0 commit comments

Comments
 (0)