Skip to content

Commit 93d9d7a

Browse files
authored
Fix/verda update code leak n new doctor (#42)
* feat(util): add version cache and comparison utility Adds VersionCache for caching GitHub release checks (24h TTL), CompareVersions for semver comparison, and PrintVersionHint for update notifications. Used by verda doctor and version-hint PostRun. * feat: add verda doctor diagnostic command Runs credential, API reachability, auth, version, binary location, and directory permission checks with human and structured output.
1 parent 8734db4 commit 93d9d7a

5 files changed

Lines changed: 654 additions & 2 deletions

File tree

internal/verda-cli/cmd/cmd.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import (
1515
"github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/availability"
1616
"github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/completion"
1717
"github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/cost"
18+
"github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/doctor"
1819
"github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/images"
1920
"github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/instancetypes"
2021
"github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/locations"
@@ -55,6 +56,14 @@ func NewRootCommand(ioStreams cmdutil.IOStreams) (*cobra.Command, *clioptions.Op
5556
return cmd.Help()
5657
},
5758
PersistentPreRunE: func(cmd *cobra.Command, args []string) error {
59+
// Agent mode always implies JSON output and no TUI. Apply
60+
// this before the credential-skip check so commands that
61+
// bypass Complete() (skills, mcp serve, auth show/use) still
62+
// get the right output mode and suppress spinners.
63+
if opts.Agent {
64+
opts.Output = "json"
65+
}
66+
5867
// Skip heavy credential resolution for commands that don't need it:
5968
// - mcp serve: defers auth to the first tool call
6069
// - auth show: diagnostic command that should work even without valid credentials
@@ -74,6 +83,21 @@ func NewRootCommand(ioStreams cmdutil.IOStreams) (*cobra.Command, *clioptions.Op
7483
}
7584
return nil
7685
},
86+
PersistentPostRun: func(cmd *cobra.Command, _ []string) {
87+
// Show version-update hint (best-effort, never fails the command).
88+
if opts.Agent || opts.Output != "table" {
89+
return
90+
}
91+
switch cmd.Name() {
92+
case "update", "doctor", "completion":
93+
return
94+
}
95+
latest, current, err := cmdutil.CheckVersion(cmd.Context())
96+
if err != nil {
97+
return
98+
}
99+
cmdutil.PrintVersionHint(ioStreams.ErrOut, latest, current)
100+
},
77101
}
78102

79103
// --version / -v flag: print rich version info.
@@ -138,6 +162,7 @@ func NewRootCommand(ioStreams cmdutil.IOStreams) (*cobra.Command, *clioptions.Op
138162
Message: "Other Commands:",
139163
Commands: []*cobra.Command{
140164
completion.NewCmdCompletion(ioStreams),
165+
doctor.NewCmdDoctor(f, ioStreams),
141166
settings.NewCmdSettings(f, ioStreams),
142167
update.NewCmdUpdate(f, ioStreams),
143168
},
@@ -167,6 +192,8 @@ func skipCredentialResolution(cmd *cobra.Command) bool {
167192
return true
168193
case pName == "skills":
169194
return true
195+
case cmd.Name() == "doctor" && pName == "verda":
196+
return true
170197
}
171198
return false
172199
}
Lines changed: 306 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,306 @@
1+
package doctor
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"io/fs"
7+
"net/http"
8+
"os"
9+
"path/filepath"
10+
"runtime"
11+
"strings"
12+
13+
"github.com/spf13/cobra"
14+
15+
cmdutil "github.com/verda-cloud/verda-cli/internal/verda-cli/cmd/util"
16+
clioptions "github.com/verda-cloud/verda-cli/internal/verda-cli/options"
17+
)
18+
19+
// checkResult holds the outcome of a single diagnostic check.
20+
type checkResult struct {
21+
Name string `json:"name"`
22+
Status string `json:"status"` // "ok", "warn", "fail", "skip"
23+
Detail string `json:"detail,omitempty"`
24+
}
25+
26+
// report is the structured output for the doctor command.
27+
type report struct {
28+
Checks []checkResult `json:"checks"`
29+
}
30+
31+
// NewCmdDoctor creates the doctor diagnostic command.
32+
func NewCmdDoctor(f cmdutil.Factory, ioStreams cmdutil.IOStreams) *cobra.Command {
33+
return &cobra.Command{
34+
Use: "doctor",
35+
Short: "Diagnose common issues",
36+
Long: cmdutil.LongDesc(`
37+
Run a series of diagnostic checks against your Verda CLI
38+
installation and report any issues found. Checks include
39+
credential configuration, API reachability, authentication,
40+
CLI version, binary location, and directory permissions.
41+
`),
42+
Example: cmdutil.Examples(`
43+
# Run all diagnostic checks
44+
verda doctor
45+
46+
# Output as JSON for scripting
47+
verda doctor -o json
48+
`),
49+
Args: cobra.NoArgs,
50+
RunE: func(cmd *cobra.Command, args []string) error {
51+
// Best-effort credential resolution so doctor works even
52+
// when credentials are bad or missing.
53+
f.Options().Complete()
54+
return runDoctor(cmd, f, ioStreams)
55+
},
56+
}
57+
}
58+
59+
func runDoctor(cmd *cobra.Command, f cmdutil.Factory, ioStreams cmdutil.IOStreams) error {
60+
ctx := cmd.Context()
61+
62+
// 1. Credentials found
63+
credResult := checkCredentials(f)
64+
65+
// 2. API reachable
66+
apiResult := checkAPIReachable(ctx, f)
67+
68+
// 3. Authentication valid (skip if creds or API failed)
69+
authResult := checkAuthentication(f, credResult, apiResult)
70+
71+
checks := []checkResult{
72+
credResult,
73+
apiResult,
74+
authResult,
75+
checkCLIVersion(ctx), // 4. CLI up to date
76+
checkBinaryInstalled(), // 5. Binary installed
77+
checkTemplatesDir(), // 6. Templates directory
78+
checkConfigDir(), // 7. Config directory
79+
}
80+
81+
r := report{Checks: checks}
82+
83+
cmdutil.DebugJSON(ioStreams.ErrOut, f.Debug(), "Doctor report:", r)
84+
85+
if wrote, err := cmdutil.WriteStructured(ioStreams.Out, f.OutputFormat(), r); wrote {
86+
return err
87+
}
88+
89+
// Human-readable table output.
90+
for _, c := range checks {
91+
symbol := statusSymbol(c.Status)
92+
detail := ""
93+
if c.Detail != "" {
94+
detail = " (" + c.Detail + ")"
95+
}
96+
_, _ = fmt.Fprintf(ioStreams.Out, " %s %s%s\n", symbol, c.Name, detail)
97+
}
98+
99+
return nil
100+
}
101+
102+
// checkCredentials verifies that a credentials file exists and contains keys.
103+
func checkCredentials(f cmdutil.Factory) checkResult {
104+
name := "Credentials found"
105+
106+
credFile := f.Options().AuthOptions.CredentialsFile
107+
if credFile == "" {
108+
var err error
109+
credFile, err = clioptions.DefaultCredentialsFilePath()
110+
if err != nil {
111+
return checkResult{Name: name, Status: "fail", Detail: err.Error()}
112+
}
113+
}
114+
115+
info, err := os.Stat(credFile)
116+
if err != nil {
117+
if os.IsNotExist(err) {
118+
return checkResult{Name: name, Status: "fail", Detail: shortPath(credFile) + " not found"}
119+
}
120+
return checkResult{Name: name, Status: "fail", Detail: err.Error()}
121+
}
122+
if info.IsDir() {
123+
return checkResult{Name: name, Status: "fail", Detail: shortPath(credFile) + " is a directory"}
124+
}
125+
126+
// File exists — check if keys are configured.
127+
auth := f.Options().AuthOptions
128+
if auth.ClientID == "" || auth.ClientSecret == "" {
129+
return checkResult{Name: name, Status: "warn", Detail: shortPath(credFile) + " exists but credentials are missing or incomplete"}
130+
}
131+
132+
return checkResult{Name: name, Status: "ok", Detail: shortPath(credFile)}
133+
}
134+
135+
// checkAPIReachable sends a HEAD request to the API server.
136+
func checkAPIReachable(ctx context.Context, f cmdutil.Factory) checkResult {
137+
name := "API reachable"
138+
server := f.Options().Server
139+
140+
ctx, cancel := context.WithTimeout(ctx, f.Options().Timeout)
141+
defer cancel()
142+
143+
req, err := http.NewRequestWithContext(ctx, http.MethodHead, server, http.NoBody)
144+
if err != nil {
145+
return checkResult{Name: name, Status: "fail", Detail: err.Error()}
146+
}
147+
148+
resp, err := f.HTTPClient().Do(req)
149+
if err != nil {
150+
return checkResult{Name: name, Status: "fail", Detail: err.Error()}
151+
}
152+
_ = resp.Body.Close()
153+
154+
// Any HTTP response (even 4xx) means the server is reachable.
155+
return checkResult{Name: name, Status: "ok", Detail: server}
156+
}
157+
158+
// checkAuthentication verifies that f.Token() returns a valid token.
159+
func checkAuthentication(f cmdutil.Factory, cred, api checkResult) checkResult {
160+
name := "Authentication valid"
161+
162+
if cred.Status != "ok" || api.Status != "ok" {
163+
return checkResult{Name: name, Status: "skip", Detail: "skipped"}
164+
}
165+
166+
token := f.Token()
167+
if token == "" {
168+
return checkResult{Name: name, Status: "fail", Detail: "could not obtain token"}
169+
}
170+
171+
return checkResult{Name: name, Status: "ok"}
172+
}
173+
174+
// checkCLIVersion compares the current version against the latest release.
175+
func checkCLIVersion(ctx context.Context) checkResult {
176+
name := "CLI up to date"
177+
178+
latest, current, err := cmdutil.CheckVersion(ctx)
179+
if err != nil {
180+
return checkResult{Name: name, Status: "warn", Detail: err.Error()}
181+
}
182+
183+
if cmdutil.CompareVersions(latest, current) > 0 {
184+
return checkResult{Name: name, Status: "warn", Detail: fmt.Sprintf("%s \u2192 %s available", current, latest)}
185+
}
186+
187+
return checkResult{Name: name, Status: "ok", Detail: current}
188+
}
189+
190+
// checkBinaryInstalled verifies the binary is in the recommended directory.
191+
func checkBinaryInstalled() checkResult {
192+
name := "Binary installed"
193+
194+
exe, err := os.Executable()
195+
if err != nil {
196+
return checkResult{Name: name, Status: "warn", Detail: err.Error()}
197+
}
198+
199+
exe, err = filepath.EvalSymlinks(exe)
200+
if err != nil {
201+
return checkResult{Name: name, Status: "warn", Detail: err.Error()}
202+
}
203+
204+
binDir, err := clioptions.VerdaBinDir()
205+
if err != nil {
206+
return checkResult{Name: name, Status: "warn", Detail: err.Error()}
207+
}
208+
209+
exeDir := filepath.Dir(exe)
210+
if exeDir == binDir {
211+
return checkResult{Name: name, Status: "ok", Detail: shortPath(exe)}
212+
}
213+
214+
return checkResult{Name: name, Status: "warn", Detail: fmt.Sprintf("running from %s, recommended: %s", shortPath(exe), shortPath(binDir))}
215+
}
216+
217+
// checkTemplatesDir checks the templates directory existence and permissions.
218+
func checkTemplatesDir() checkResult {
219+
name := "Templates directory"
220+
221+
dir, err := cmdutil.TemplatesBaseDir()
222+
if err != nil {
223+
return checkResult{Name: name, Status: "warn", Detail: err.Error()}
224+
}
225+
226+
return checkDirPerms(name, dir)
227+
}
228+
229+
// checkConfigDir checks the config directory existence and permissions.
230+
func checkConfigDir() checkResult {
231+
name := "Config directory"
232+
233+
dir, err := clioptions.VerdaDir()
234+
if err != nil {
235+
return checkResult{Name: name, Status: "warn", Detail: err.Error()}
236+
}
237+
238+
return checkDirPerms(name, dir)
239+
}
240+
241+
// checkDirPerms checks that a directory exists and has secure permissions.
242+
// If the directory doesn't exist, it returns ok (not an error — it may not
243+
// have been created yet). On Windows, permission checks are skipped.
244+
func checkDirPerms(name, dir string) checkResult {
245+
info, err := os.Stat(dir)
246+
if err != nil {
247+
if os.IsNotExist(err) {
248+
return checkResult{Name: name, Status: "ok", Detail: shortPath(dir) + " (not created yet)"}
249+
}
250+
return checkResult{Name: name, Status: "warn", Detail: err.Error()}
251+
}
252+
253+
if runtime.GOOS == "windows" {
254+
return checkResult{Name: name, Status: "ok", Detail: shortPath(dir)}
255+
}
256+
257+
if !info.IsDir() {
258+
return checkResult{Name: name, Status: "warn", Detail: shortPath(dir) + " is not a directory"}
259+
}
260+
261+
if hasLoosePerms(info) {
262+
return checkResult{
263+
Name: name,
264+
Status: "warn",
265+
Detail: fmt.Sprintf("%s has permissions %s, recommended: 0700", shortPath(dir), info.Mode().Perm()),
266+
}
267+
}
268+
269+
return checkResult{Name: name, Status: "ok", Detail: shortPath(dir)}
270+
}
271+
272+
// hasLoosePerms reports whether group or other permission bits are set.
273+
func hasLoosePerms(info fs.FileInfo) bool {
274+
return info.Mode().Perm()&0o077 != 0
275+
}
276+
277+
// statusSymbol returns a human-readable status indicator.
278+
func statusSymbol(status string) string {
279+
switch status {
280+
case "ok":
281+
return "\u2713" // ✓
282+
case "warn":
283+
return "!"
284+
case "fail":
285+
return "\u2717" // ✗
286+
case "skip":
287+
return "-"
288+
default:
289+
return "?"
290+
}
291+
}
292+
293+
// shortPath replaces the user's home directory prefix with ~.
294+
func shortPath(p string) string {
295+
home, err := os.UserHomeDir()
296+
if err != nil {
297+
return p
298+
}
299+
if strings.HasPrefix(p, home+string(filepath.Separator)) {
300+
return "~" + p[len(home):]
301+
}
302+
if p == home {
303+
return "~"
304+
}
305+
return p
306+
}

internal/verda-cli/cmd/update/update.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -441,8 +441,8 @@ func updateInstalledSkills(ctx context.Context, newBinary string, ioStreams cmdu
441441
return
442442
}
443443

444-
args := make([]string, 0, 4+len(state.Agents))
445-
args = append(args, "--agent", "skills", "install", "--force")
444+
args := make([]string, 0, 6+len(state.Agents))
445+
args = append(args, "--agent", "-o", "json", "skills", "install", "--force")
446446
args = append(args, state.Agents...)
447447

448448
cmd := exec.CommandContext(ctx, newBinary, args...) //nolint:gosec // newBinary is the just-installed verda binary

0 commit comments

Comments
 (0)