fix: KEEP-1049 compare CLI version against server floor, stop showing API keys as identities

cmd/auth/login.go

@@ -35,6 +35,17 @@ var FetchTokenInfoFunc = func(host, token string) (internalauth.TokenInfo, error
 	return internalauth.FetchTokenInfo(host, token)
 }
 
+// credentialIdentity describes info for "logged in as %s" style messages.
+// API keys have no real identity to show - info.Email holds a truncated key
+// prefix for lack of one - so it is named as a credential rather than
+// presented as though the key were an account.
+func credentialIdentity(info internalauth.TokenInfo) string {
+	if info.Method == internalauth.AuthMethodAPIKey {
+		return "API key " + info.Email
+	}
+	return info.Email
+}
+
 func NewLoginCmd(f *cmdutil.Factory) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "login",
@@ -78,7 +89,7 @@ See also: kh auth status, kh auth logout`,
 					if info, infoErr := FetchTokenInfoFunc(host, entry.Token); infoErr == nil {
 						fmt.Fprintf(f.IOStreams.Out,
 							"Already logged in to %s as %s\nUse --force to replace the stored API key.\n",
-							host, info.Email)
+							host, credentialIdentity(info))
 						return nil
 					}
 				}
@@ -111,7 +122,7 @@ See also: kh auth status, kh auth logout`,
 			if err != nil {
 				fmt.Fprintf(f.IOStreams.Out, "Logged in to %s\n", host)
 			} else {
-				fmt.Fprintf(f.IOStreams.Out, "Logged in to %s as %s\n", host, info.Email)
+				fmt.Fprintf(f.IOStreams.Out, "Logged in to %s as %s\n", host, credentialIdentity(info))
 			}
 
 			// The device flow creates a real organization API key rather than a

cmd/auth/login_apikey_test.go

@@ -56,6 +56,37 @@ func TestLoginCmd_SkipsWhenStoredTokenIsValid(t *testing.T) {
 	}
 }
 
+// fetchAPIKeyInfo sets Email to a truncated key prefix for lack of a real
+// identity. This pins that the already-logged-in notice names it as a
+// credential rather than presenting the prefix as though it were an account.
+func TestLoginCmd_AlreadyLoggedInNamesAPIKeyAsCredential(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	if err := config.SetHostToken(testHost, "kh_already_stored"); err != nil {
+		t.Fatalf("seeding token: %v", err)
+	}
+
+	ios, buf, _, _ := iostreams.Test()
+	auth.DeviceLoginFunc = func(string, *iostreams.IOStreams) (string, error) {
+		t.Fatal("DeviceLogin must not run when a valid token is stored")
+		return "", nil
+	}
+	auth.SetTokenFunc = func(string, string) error { return nil }
+	auth.FetchTokenInfoFunc = func(_, token string) (internalauth.TokenInfo, error) {
+		return internalauth.TokenInfo{Email: "kh_EU7Fc1Xi...", Method: internalauth.AuthMethodAPIKey}, nil
+	}
+
+	cmd := auth.NewLoginCmd(&cmdutil.Factory{IOStreams: ios})
+	cmd.SetArgs([]string{})
+	if err := cmd.Execute(); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	out := buf.String()
+	if !strings.Contains(out, "as API key kh_EU7Fc1Xi...") {
+		t.Errorf("expected the key prefix named as an API key, got: %q", out)
+	}
+}
+
 func TestLoginCmd_ForceReplacesStoredToken(t *testing.T) {
 	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
 	if err := config.SetHostToken(testHost, "kh_already_stored"); err != nil {

cmd/auth/status.go

@@ -85,10 +85,19 @@ func NewStatusCmd(f *cmdutil.Factory) *cobra.Command {
 				Host:      host,
 			}
 
+			// API keys have no real identity to show - info.Email holds a
+			// truncated key prefix for lack of one - so the row is labeled as
+			// a credential rather than presented as though the key were a
+			// user account.
+			userLabel := "User"
+			if info.Method == internalauth.AuthMethodAPIKey {
+				userLabel = "Credential"
+			}
+
 			p := output.NewPrinter(f.IOStreams, cmd)
 			return p.PrintData(data, func(tw table.Writer) {
 				tw.AppendRow(table.Row{"Host", host})
-				tw.AppendRow(table.Row{"User", info.Email})
+				tw.AppendRow(table.Row{userLabel, info.Email})
 				tw.AppendRow(table.Row{"Organization", info.OrgName})
 				tw.AppendRow(table.Row{"Role", info.Role})
 				if expiresAt != "" {

cmd/auth/status_test.go

@@ -97,6 +97,70 @@ func TestStatusCmd_APIKeyMethod(t *testing.T) {
 	}
 }
 
+// fetchAPIKeyInfo has no real identity for an API key, so it sets Email to a
+// truncated key prefix (e.g. "kh_EU7Fc1Xi..."). This pins that the table
+// stops labeling that prefix "User", which reads as though the key were an
+// account, and uses "Credential" for API-key auth instead.
+func TestStatusCmd_APIKeyLabeledAsCredentialNotUser(t *testing.T) {
+	ios, buf, _, _ := iostreams.Test()
+
+	auth.ResolveTokenFunc = func(host string) (internalauth.ResolvedToken, error) {
+		return internalauth.ResolvedToken{Token: "kh_EU7Fc1Xi", Method: internalauth.AuthMethodAPIKey, Host: host}, nil
+	}
+	auth.FetchTokenInfoFunc = func(host, token string) (internalauth.TokenInfo, error) {
+		return internalauth.TokenInfo{
+			Email:   "kh_EU7Fc1Xi...",
+			OrgName: "My Org",
+			Role:    "api-key",
+			Method:  internalauth.AuthMethodAPIKey,
+		}, nil
+	}
+
+	f := &cmdutil.Factory{IOStreams: ios}
+	cmd := auth.NewStatusCmd(f)
+	cmd.SetArgs([]string{})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	out := buf.String()
+	if !strings.Contains(out, "Credential") {
+		t.Errorf("expected the row labeled Credential for API-key auth, got: %q", out)
+	}
+	if strings.Contains(out, "User") {
+		t.Errorf("must not label the key prefix as User, got: %q", out)
+	}
+}
+
+func TestStatusCmd_SessionLabeledAsUser(t *testing.T) {
+	ios, buf, _, _ := iostreams.Test()
+
+	auth.ResolveTokenFunc = func(host string) (internalauth.ResolvedToken, error) {
+		return internalauth.ResolvedToken{Token: "tok", Method: internalauth.AuthMethodToken, Host: host}, nil
+	}
+	auth.FetchTokenInfoFunc = func(host, token string) (internalauth.TokenInfo, error) {
+		return internalauth.TokenInfo{
+			Email:   "user@example.com",
+			OrgName: "My Org",
+			Role:    "owner",
+			Method:  internalauth.AuthMethodToken,
+		}, nil
+	}
+
+	f := &cmdutil.Factory{IOStreams: ios}
+	cmd := auth.NewStatusCmd(f)
+	cmd.SetArgs([]string{})
+
+	if err := cmd.Execute(); err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	if !strings.Contains(buf.String(), "User") {
+		t.Errorf("expected the row labeled User for session auth, got: %q", buf.String())
+	}
+}
+
 func TestStatusCmd_JSONOutput(t *testing.T) {
 	ios, buf, _, _ := iostreams.Test()
 

cmd/doctor/doctor.go

@@ -368,14 +368,47 @@ func checkChains(ctx context.Context, f *cmdutil.Factory) CheckResult {
 	return CheckResult{Status: "pass", Message: fmt.Sprintf("%d chains available", len(chains))}
 }
 
-// checkCLIVersion reports the current CLI version. Network-based latest-version
-// checking is deferred to Phase 24; this check is always instantaneous.
-func checkCLIVersion(_ context.Context, f *cmdutil.Factory) CheckResult {
+// checkCLIVersion reports the current CLI version, and warns if it is older
+// than the floor the server advertises via the KH-Minimum-CLI-Version
+// response header (see khhttp.MinimumVersionHeader).
+func checkCLIVersion(ctx context.Context, f *cmdutil.Factory) CheckResult {
 	v := f.AppVersion
 	if v == "" || v == "dev" || strings.HasPrefix(v, "v0.0.0") {
 		return CheckResult{Status: "warn", Message: "development build"}
 	}
-	return CheckResult{Status: "pass", Message: fmt.Sprintf("v%s", strings.TrimPrefix(v, "v"))}
+	current := strings.TrimPrefix(v, "v")
+	localVersion := "v" + current
+
+	host, err := getHost(f)
+	if err != nil {
+		return CheckResult{Status: "pass", Message: localVersion}
+	}
+	client, err := getHTTPClient(f)
+	if err != nil {
+		return CheckResult{Status: "pass", Message: localVersion}
+	}
+
+	url := khhttp.BuildBaseURL(host) + "/api/health"
+	resp, err := doGet(ctx, client, host, url)
+	if err != nil {
+		// No server to compare against -- report the local version rather
+		// than failing a check whose job is reachability elsewhere (checkAPI).
+		return CheckResult{Status: "pass", Message: localVersion}
+	}
+	defer resp.Body.Close()
+	_, _ = io.Copy(io.Discard, resp.Body)
+
+	minimum := resp.Header.Get(khhttp.MinimumVersionHeader)
+	if minimum == "" {
+		return CheckResult{Status: "pass", Message: localVersion}
+	}
+	if khhttp.SemverLessThan(current, minimum) {
+		return CheckResult{
+			Status:  "warn",
+			Message: fmt.Sprintf("%s is outdated; minimum required is %s. Run: kh update", localVersion, minimum),
+		}
+	}
+	return CheckResult{Status: "pass", Message: localVersion}
 }
 
 // abbreviateAddr shortens a long address like 0xABCD...1234.

cmd/doctor/doctor_version_test.go

@@ -0,0 +1,106 @@
+package doctor_test
+
+// checkCLIVersion used to only print the local version string, unconditionally
+// pass, and never compare against anything -- the server had no way to tell an
+// out-of-date CLI that it was behind. These tests pin the comparison against
+// the KH-Minimum-CLI-Version header the server advertises on /api/*
+// (khhttp.MinimumVersionHeader).
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/keeperhub/cli/cmd/doctor"
+	"github.com/keeperhub/cli/internal/config"
+	khhttp "github.com/keeperhub/cli/internal/http"
+	"github.com/keeperhub/cli/pkg/cmdutil"
+	"github.com/keeperhub/cli/pkg/iostreams"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// cliVersionLine returns the doctor output line reporting the CLI Version check.
+func cliVersionLine(out string) string {
+	for _, line := range strings.Split(out, "\n") {
+		if strings.Contains(line, "CLI Version:") {
+			return line
+		}
+	}
+	return ""
+}
+
+// versionDoctorFactory builds a Factory pinned to appVersion, backed by a test
+// server that advertises minimumHeader (empty means the header is omitted).
+func versionDoctorFactory(ios *iostreams.IOStreams, appVersion, minimumHeader string) *cmdutil.Factory {
+	svr := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if minimumHeader != "" && strings.HasPrefix(r.URL.Path, "/api/") {
+			w.Header().Set(khhttp.MinimumVersionHeader, minimumHeader)
+		}
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{}`))
+	}))
+	return &cmdutil.Factory{
+		AppVersion: appVersion,
+		IOStreams:  ios,
+		Config: func() (config.Config, error) {
+			return config.Config{DefaultHost: svr.URL}, nil
+		},
+		HTTPClient: func() (*khhttp.Client, error) {
+			return khhttp.NewClient(khhttp.ClientOptions{
+				AppVersion: appVersion,
+				IOStreams:  ios,
+			}), nil
+		},
+	}
+}
+
+func TestDoctorCmd_CLIVersionPassesWhenServerAdvertisesNoMinimum(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	ios, outBuf, _, _ := iostreams.Test()
+	tc := doctor.NewTestableCmd(versionDoctorFactory(ios, "1.2.3", ""))
+	require.NoError(t, tc.Execute([]string{}))
+
+	line := cliVersionLine(outBuf.String())
+	assert.Contains(t, line, "pass")
+	assert.Contains(t, line, "v1.2.3")
+}
+
+func TestDoctorCmd_CLIVersionWarnsWhenBelowServerMinimum(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	ios, outBuf, _, _ := iostreams.Test()
+	tc := doctor.NewTestableCmd(versionDoctorFactory(ios, "0.3.0", "0.11.1"))
+	require.NoError(t, tc.Execute([]string{}))
+
+	line := cliVersionLine(outBuf.String())
+	assert.Contains(t, line, "warn")
+	assert.Contains(t, line, "outdated")
+	assert.Contains(t, line, "0.11.1")
+	assert.Contains(t, line, "Run: kh update")
+}
+
+func TestDoctorCmd_CLIVersionPassesWhenAtServerMinimum(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	ios, outBuf, _, _ := iostreams.Test()
+	tc := doctor.NewTestableCmd(versionDoctorFactory(ios, "0.11.1", "0.11.1"))
+	require.NoError(t, tc.Execute([]string{}))
+
+	line := cliVersionLine(outBuf.String())
+	assert.Contains(t, line, "pass")
+	assert.NotContains(t, line, "outdated")
+}
+
+func TestDoctorCmd_CLIVersionReportsDevBuildWithoutComparing(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	ios, outBuf, _, _ := iostreams.Test()
+	// A minimum far above any real release: if dev builds were compared,
+	// this would incorrectly warn.
+	tc := doctor.NewTestableCmd(versionDoctorFactory(ios, "dev", "99.0.0"))
+	require.NoError(t, tc.Execute([]string{}))
+
+	line := cliVersionLine(outBuf.String())
+	assert.Contains(t, line, "development build")
+	assert.NotContains(t, line, "outdated")
+}

internal/http/version.go

@@ -8,6 +8,11 @@ import (
 	"strings"
 )
 
+// MinimumVersionHeader is the response header the server uses to advertise
+// the oldest CLI version it considers supported (see keeperhub's
+// lib/cli-version.ts). Shared so every call site checks the same header.
+const MinimumVersionHeader = "KH-Minimum-CLI-Version"
+
 // semverLessThan returns true if current is strictly less than minimum.
 // Returns false if current is "dev" or unparseable -- dev builds never trigger warnings.
 func semverLessThan(current, minimum string) bool {
@@ -62,7 +67,7 @@ func checkVersion(current string, resp *http.Response, errOut io.Writer) {
 	if resp == nil {
 		return
 	}
-	minimum := resp.Header.Get("KH-Minimum-CLI-Version")
+	minimum := resp.Header.Get(MinimumVersionHeader)
 	if minimum == "" {
 		return
 	}