feat(credentials): store CLI secrets in the OS keyring (AST-164107)

go.mod

@@ -29,6 +29,7 @@ require (
 	github.com/stretchr/testify v1.11.1
 	github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80
 	github.com/xeipuuv/gojsonschema v1.2.0
+	github.com/zalando/go-keyring v0.2.8
 	golang.org/x/crypto v0.55.0
 	golang.org/x/sync v0.22.0
 	golang.org/x/text v0.41.0
@@ -46,10 +47,12 @@ require (
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/clipperhouse/displaywidth v0.10.0 // indirect
 	github.com/clipperhouse/uax29/v2 v2.6.0 // indirect
+	github.com/danieljoos/wincred v1.2.3 // indirect
 	github.com/distribution/distribution/v3 v3.1.1 // indirect
 	github.com/docker/docker v28.0.3+incompatible // indirect
 	github.com/docker/go-events v0.0.0-20250808211157-605354379745 // indirect
 	github.com/edsrzf/mmap-go v1.2.0 // indirect
+	github.com/godbus/dbus/v5 v5.2.2 // indirect
 	github.com/golang/snappy v1.0.0 // indirect
 	github.com/google/jsonschema-go v0.4.3 // indirect
 	github.com/klauspost/cpuid/v2 v2.3.0 // indirect
@@ -335,3 +338,5 @@ replace github.com/docker/go-connections => github.com/docker/go-connections v0.
 replace github.com/opencontainers/selinux => github.com/opencontainers/selinux v1.13.0
 
 replace github.com/opencontainers/runtime-spec => github.com/opencontainers/runtime-spec v1.2.0
+
+replace github.com/zalando/go-keyring => github.com/checkmarx/go-keyring v0.0.0-20260812105354-1168609d33ac

go.sum

@@ -223,6 +223,8 @@ github.com/charmbracelet/x/term v0.2.2 h1:xVRT/S2ZcKdhhOuSP4t5cLi5o+JxklsoEObBSg
 github.com/charmbracelet/x/term v0.2.2/go.mod h1:kF8CY5RddLWrsgVwpw4kAa6TESp6EB5y3uxGLeCqzAI=
 github.com/checkmarx/2ms/v3 v3.21.0 h1:EcabeDypNMsSidISQbziZ062HjMZQ+Hm/uOJ5AOxK8o=
 github.com/checkmarx/2ms/v3 v3.21.0/go.mod h1:e8f4F94MZ+iCetR/G3aw7nXdPe6TgPI92Zzk/NG1l0o=
+github.com/checkmarx/go-keyring v0.0.0-20260812105354-1168609d33ac h1:q/EiuayTYiRZdeCSeauE5+G8kd3SkMfLEmMoaZDtcLI=
+github.com/checkmarx/go-keyring v0.0.0-20260812105354-1168609d33ac/go.mod h1:tsMo+VpRq5NGyKfxoBVjCuMrG47yj8cmakZDO5QGii0=
 github.com/chromedp/cdproto v0.0.0-20230802225258-3cf4e6d46a89/go.mod h1:GKljq0VrfU4D5yc+2qA6OVr8pmO/MBbPEWqWQ/oqGEs=
 github.com/chromedp/chromedp v0.9.2/go.mod h1:LkSXJKONWTCHAfQasKFUZI+mxqS4tZqhmtGzzhLsnLs=
 github.com/chromedp/sysutil v1.0.0/go.mod h1:kgWmDdq8fTzXYcKIBqIYvRRTnYb9aNS9moAV0xufSww=
@@ -284,6 +286,8 @@ github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
 github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
 github.com/cyphar/filepath-securejoin v0.6.1 h1:5CeZ1jPXEiYt3+Z6zqprSAgSWiggmpVyciv8syjIpVE=
 github.com/cyphar/filepath-securejoin v0.6.1/go.mod h1:A8hd4EnAeyujCJRrICiOWqjS1AX0a9kM5XL+NwKoYSc=
+github.com/danieljoos/wincred v1.2.3 h1:v7dZC2x32Ut3nEfRH+vhoZGvN72+dQ/snVXo/vMFLdQ=
+github.com/danieljoos/wincred v1.2.3/go.mod h1:6qqX0WNrS4RzPZ1tnroDzq9kY3fu1KwE7MRLQK4X0bs=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM=
@@ -433,6 +437,8 @@ github.com/gobwas/httphead v0.1.0/go.mod h1:O/RXo79gxV8G+RqlR/otEwx4Q36zl9rqC5u1
 github.com/gobwas/pool v0.2.1/go.mod h1:q8bcK0KcYlCgd9e7WYLm9LpyS+YeLd8JVDW6WezmKEw=
 github.com/gobwas/ws v1.2.1/go.mod h1:hRKAFb8wOxFROYNsT1bqfWnhX+b5MFeJM9r2ZSwg/KY=
 github.com/godbus/dbus/v5 v5.0.4/go.mod h1:xhWf0FNVPg57R7Z0UbKHbJfkEywrmjJnf7w5xrFpKfA=
+github.com/godbus/dbus/v5 v5.2.2 h1:TUR3TgtSVDmjiXOgAAyaZbYmIeP3DPkld3jgKGV8mXQ=
+github.com/godbus/dbus/v5 v5.2.2/go.mod h1:3AAv2+hPq5rdnr5txxxRwiGjPXamgoIHgz9FPBfOp3c=
 github.com/gofrs/flock v0.13.0 h1:95JolYOvGMqeH31+FC7D2+uULf6mG61mEZ/A8dRYMzw=
 github.com/gofrs/flock v0.13.0/go.mod h1:jxeyy9R1auM5S6JYDBhDt+E2TCo7DkratH4Pgi8P+Z0=
 github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ=

internal/commands/agenthooks/mcp/bridge.go

@@ -14,6 +14,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/checkmarx/ast-cli/internal/credentialstore"
 	"github.com/checkmarx/ast-cli/internal/logger"
 	commonParams "github.com/checkmarx/ast-cli/internal/params"
 	"github.com/checkmarx/ast-cli/internal/wrappers"
@@ -28,8 +29,8 @@ import (
 // binary guaranteed present, instead of bash/node/python (none of which are
 // guaranteed across Windows/macOS/Linux or on a native, Bun-based Claude install).
 //
-// It reads the credential cx already resolved (env CX_APIKEY / cx config, loaded
-// at startup), derives the realm-scoped URL from the credential's JWT `iss`
+// It reads the credential cx already resolved (OS keyring / env CX_APIKEY / cx
+// config, loaded at startup), derives the realm-scoped URL from the credential's JWT `iss`
 // claim, and forwards newline-delimited JSON-RPC between stdin/stdout and the
 // remote MCP's Streamable HTTP endpoint (application/json + text/event-stream,
 // Mcp-Session-Id). The credential is sent ONLY in the Authorization header (the
@@ -135,17 +136,22 @@ var (
 // productionResolveAPIKey reads it. viper itself has no internal locking.
 var configMu sync.Mutex
 
-// productionResolveAPIKey returns the credential cx resolved (CX_APIKEY env / cx
-// config / active session), falling back to CHECKMARX_API_KEY for parity with the
-// previous Python bridge. Callers that need a credential written AFTER startup must
-// call reloadConfig() first (viper is a one-shot startup snapshot). The viper read
-// is guarded by configMu so it never races a concurrent reloadConfig.
+// productionResolveAPIKey returns the credential cx resolved (keyring /
+// CX_APIKEY env / cx config), falling back to CHECKMARX_API_KEY for parity with
+// the previous Python bridge. The credentialstore read covers the keyring and
+// env layers; reloadConfig() keeps the viper snapshot fresh for other keys.
 func productionResolveAPIKey() string {
+	k, err := credentialstore.Resolve(credentialstore.CredentialAPIKey)
+	if err == nil {
+		if k = strings.TrimSpace(k); k != "" {
+			return k
+		}
+	}
 	configMu.Lock()
-	k := strings.TrimSpace(viper.GetString(commonParams.AstAPIKey))
+	v := strings.TrimSpace(viper.GetString(commonParams.AstAPIKey))
 	configMu.Unlock()
-	if k != "" {
-		return k
+	if v != "" {
+		return v
 	}
 	if k := strings.TrimSpace(os.Getenv("CHECKMARX_API_KEY")); k != "" {
 		return k
@@ -169,7 +175,7 @@ Intended to be launched by an AI coding assistant as an MCP server:
     }
   }
 
-The credential is read from cx config (or CX_APIKEY). The realm-scoped URL is
+The credential is read from the OS keyring, CX_APIKEY, or cx config. The realm-scoped URL is
 resolved by, in order: the --mcp-url flag, the CX_MCP_URL env var, the
 authoritative "ast-base-url" claim from the exchanged access token (works for
 any region/on-prem), then an offline IAM->AST host swap. Override with --mcp-url

internal/commands/agenthooks/mcp/main_test.go

@@ -0,0 +1,29 @@
+package mcp
+
+import (
+	"log"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/checkmarx/ast-cli/internal/credentialstore"
+	"github.com/checkmarx/ast-cli/internal/params"
+)
+
+// TestMain isolates bridge tests from the real OS keyring and config file.
+func TestMain(m *testing.M) {
+	testConfigDir, err := os.MkdirTemp("", "cx-mcp-test-config")
+	if err != nil {
+		log.Fatalf("failed to create test config dir: %v", err)
+	}
+	configPath := filepath.Join(testConfigDir, "checkmarxcli.yaml")
+	if err := os.WriteFile(configPath, nil, 0o600); err != nil {
+		log.Fatalf("failed to seed test config file: %v", err)
+	}
+	_ = os.Setenv(credentialstore.KeyringModeEnvVar, "disabled")
+	_ = os.Setenv(params.ConfigFilePathEnv, configPath)
+	credentialstore.ResetForTest()
+	exitVal := m.Run()
+	_ = os.RemoveAll(testConfigDir)
+	os.Exit(exitVal)
+}

internal/commands/auth.go

@@ -5,6 +5,7 @@ import (
 	"log"
 
 	"github.com/MakeNowJust/heredoc"
+	"github.com/checkmarx/ast-cli/internal/credentialstore"
 	"github.com/checkmarx/ast-cli/internal/logger"
 	"github.com/checkmarx/ast-cli/internal/params"
 	"github.com/checkmarx/ast-cli/internal/wrappers"
@@ -120,8 +121,16 @@ func NewAuthCommand(authWrapper wrappers.AuthWrapper, telemetryWrapper wrappers.
 func validLogin(telemetryWrapper wrappers.TelemetryWrapper) func(cmd *cobra.Command, args []string) error {
 	return func(cmd *cobra.Command, args []string) error {
 		clientID := viper.GetString(params.AccessKeyIDConfigKey)
-		clientSecret := viper.GetString(params.AccessKeySecretConfigKey)
-		apiKey := viper.GetString(params.AstAPIKey)
+		clientSecret, secretErr := credentialstore.Resolve(credentialstore.CredentialClientSecret)
+		if secretErr != nil {
+			logger.PrintIfVerbose(fmt.Sprintf("auth validate: resolving client secret: %v", secretErr))
+			clientSecret = ""
+		}
+		apiKey, keyErr := credentialstore.Resolve(credentialstore.CredentialAPIKey)
+		if keyErr != nil {
+			logger.PrintIfVerbose(fmt.Sprintf("auth validate: resolving api key: %v", keyErr))
+			apiKey = ""
+		}
 		if (clientID != "" && clientSecret != "") || apiKey != "" {
 			authWrapper := wrappers.NewAuthHTTPWrapper()
 			authWrapper.SetPath(viper.GetString(params.ScansPathKey))

internal/commands/auth_login.go

@@ -6,6 +6,7 @@ import (
 	"os"
 
 	"github.com/MakeNowJust/heredoc"
+	"github.com/checkmarx/ast-cli/internal/credentialstore"
 	"github.com/checkmarx/ast-cli/internal/logger"
 	"github.com/checkmarx/ast-cli/internal/params"
 	"github.com/checkmarx/ast-cli/internal/wrappers"
@@ -27,13 +28,13 @@ func newAuthLoginCommand() *cobra.Command {
 		Use:   "login",
 		Short: "Authenticate to Checkmarx One via browser-based OAuth",
 		Long: "Opens the default browser, walks the user through the Checkmarx One IAM login " +
-			"(including MFA), and saves the resulting refresh token to the config file's cx_apikey " +
-			"field — the same credential slot cx configure writes to, so every other command picks " +
-			"it up automatically.\n\n" +
+			"(including MFA), and stores the resulting refresh token securely in the OS keyring " +
+			"under the api-key credential slot — the same slot cx configure writes to, so every " +
+			"other command picks it up automatically.\n\n" +
 			"Requires --tenant and --base-uri (or --base-auth-uri). Pass them as flags, or run " +
 			"cx auth login with none and it prompts for the missing ones like cx configure.",
 		Example: heredoc.Doc(`
-			# With flags — saves the refresh token to ~/.checkmarx/checkmarxcli.yaml
+			# With flags — stores the refresh token in the OS keyring
 			$ cx auth login --base-uri https://<region>.ast.checkmarx.net --tenant my-tenant
 
 			# No flags — prompts for base URI / tenant, then opens the browser
@@ -83,7 +84,7 @@ func runAuthLogin(cmd *cobra.Command, _ []string) error {
 		return err
 	}
 
-	return persistYamlLogin(cmd, tokens.RefreshToken)
+	return persistLogin(cmd, tokens.RefreshToken)
 }
 
 // connectionFlagsProvided reports whether any connection detail was passed as a flag.
@@ -93,18 +94,24 @@ func connectionFlagsProvided(cmd *cobra.Command) bool {
 		cmd.Flags().Changed(params.TenantFlag)
 }
 
-// persistYamlLogin saves the refresh token to cx_apikey; never echoes it to stdout.
-func persistYamlLogin(cmd *cobra.Command, refreshToken string) error {
-	configPath, err := configuration.GetConfigFilePath()
-	if err != nil {
-		return errors.Wrap(err, "failed to resolve config file path")
+// persistLogin stores the refresh token under the api-key credential slot;
+// never echoes it to stdout.
+func persistLogin(cmd *cobra.Command, refreshToken string) error {
+	if err := credentialstore.Default().Store(context.Background(), credentialstore.CredentialAPIKey, refreshToken); err != nil {
+		return errors.Wrap(err, "failed to save refresh token")
 	}
-	if err := configuration.SafeWriteSingleConfigKeyString(configPath, params.AstAPIKey, refreshToken); err != nil {
-		return errors.Wrap(err, "failed to save refresh token to config file")
+	if !credentialstore.Default().StoresInConfigFile() {
+		if removeErr := credentialstore.Default().RemoveConfigFileEntry(credentialstore.CredentialAPIKey); removeErr != nil {
+			logger.PrintIfVerbose(fmt.Sprintf("failed to remove old refresh token from config file: %v", removeErr))
+		}
 	}
-	// Restrict to owner-only; best-effort no-op on Windows.
-	if chErr := os.Chmod(configPath, configFilePerm); chErr != nil {
-		logger.PrintIfVerbose(fmt.Sprintf("failed to restrict config file permissions: %v", chErr))
+	// Restrict a pre-existing config file to owner-only; best-effort no-op on
+	// Windows and when the file does not exist.
+	configPath, err := configuration.GetConfigFilePath()
+	if err == nil {
+		if chErr := os.Chmod(configPath, configFilePerm); chErr != nil {
+			logger.PrintIfVerbose(fmt.Sprintf("failed to restrict config file permissions: %v", chErr))
+		}
 	}
 	_, _ = fmt.Fprintln(cmd.OutOrStdout(), "Successfully authenticated to Checkmarx One server!")
 	return nil

internal/commands/auth_login_test.go

@@ -8,8 +8,10 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/checkmarx/ast-cli/internal/credentialstore"
 	"github.com/checkmarx/ast-cli/internal/params"
 	"github.com/checkmarx/ast-cli/internal/wrappers/configuration"
+	"github.com/checkmarx/ast-cli/internal/wrappers/mock"
 
 	"github.com/spf13/cobra"
 	"github.com/spf13/viper"
@@ -18,8 +20,6 @@ import (
 // The full runAuthLogin (browser + network) is out of scope; these cover the
 // deterministic pieces: persistLogin and runAuthLogout.
 
-// swapDefaultStore swaps credentialstore.Default for a mock and restores it.
-
 // withTempConfigDir sandboxes viper at a temp config file and clears CX_APIKEY.
 func withTempConfigDir(t *testing.T) string {
 	t.Helper()
@@ -31,6 +31,20 @@ func withTempConfigDir(t *testing.T) string {
 	return dir
 }
 
+// swapCredentialResolver binds a mock-backed resolver to the sandbox config
+// path so credential reads/writes never reach the real OS keyring.
+func swapCredentialResolver(t *testing.T) *mock.CredentialStoreMock {
+	t.Helper()
+	configPath, err := configuration.GetConfigFilePath()
+	if err != nil {
+		t.Fatalf("GetConfigFilePath failed: %v", err)
+	}
+	store := mock.NewCredentialStoreMock()
+	credentialstore.SetDefaultResolverForTest(credentialstore.NewResolver(configPath, credentialstore.PolicyAuto, store))
+	t.Cleanup(credentialstore.ResetForTest)
+	return store
+}
+
 // newBufferedCmd returns a cobra command whose stdout/stderr are captured.
 func newBufferedCmd() (*cobra.Command, *bytes.Buffer, *bytes.Buffer) {
 	cmd := &cobra.Command{}
@@ -100,6 +114,7 @@ func TestConnectionFlagsProvided(t *testing.T) {
 // Logout clears cx_apikey and is idempotent.
 func TestRunAuthLogout_ClearsYaml(t *testing.T) {
 	dir := withTempConfigDir(t)
+	swapCredentialResolver(t)
 	configPath := filepath.Join(dir, "checkmarxcli.yaml")
 	if err := configuration.SafeWriteSingleConfigKeyString(configPath, params.AstAPIKey, "stored-token"); err != nil {
 		t.Fatalf("setup yaml write failed: %v", err)
@@ -145,35 +160,37 @@ func TestRunAuthLogout_DoesNotClearClientCredentials(t *testing.T) {
 	}
 }
 
-// persistYamlLogin saves the refresh token to the config file.
-func TestPersistYamlLogin_SavesTokenAndPrintsSuccess(t *testing.T) {
+// persistLogin saves the refresh token to the credential store.
+func TestPersistLogin_SavesTokenAndPrintsSuccess(t *testing.T) {
 	_ = withTempConfigDir(t)
+	store := swapCredentialResolver(t)
 	cmd, out, _ := newBufferedCmd()
 	refreshToken := "refresh-token-abc123"
 
-	if err := persistYamlLogin(cmd, refreshToken); err != nil {
-		t.Fatalf("persistYamlLogin failed: %v", err)
+	if err := persistLogin(cmd, refreshToken); err != nil {
+		t.Fatalf("persistLogin failed: %v", err)
 	}
 
-	// Check token was saved to YAML
-	if got := readYamlAPIKey(t); got != refreshToken {
-		t.Errorf("expected token saved to yaml, got %q want %q", got, refreshToken)
+	if got := store.Store[credentialstore.CredentialAPIKey]; got != refreshToken {
+		t.Errorf("expected token saved to credential store, got %q want %q", got, refreshToken)
+	}
+	if got := readYamlAPIKey(t); got != "" {
+		t.Errorf("expected legacy yaml entry scrubbed, got %q", got)
 	}
 
-	// Check success message was printed
 	if !strings.Contains(out.String(), "Successfully authenticated to Checkmarx One server!") {
 		t.Errorf("expected success message, got: %q", out.String())
 	}
 }
 
-// persistYamlLogin does not echo the token to stdout
-func TestPersistYamlLogin_DoesNotEchoToken(t *testing.T) {
+// persistLogin does not echo the token to stdout
+func TestPersistLogin_DoesNotEchoToken(t *testing.T) {
 	_ = withTempConfigDir(t)
 	cmd, out, _ := newBufferedCmd()
 	refreshToken := "secret-refresh-token-12345"
 
-	if err := persistYamlLogin(cmd, refreshToken); err != nil {
-		t.Fatalf("persistYamlLogin failed: %v", err)
+	if err := persistLogin(cmd, refreshToken); err != nil {
+		t.Fatalf("persistLogin failed: %v", err)
 	}
 
 	output := out.String()
@@ -182,8 +199,8 @@ func TestPersistYamlLogin_DoesNotEchoToken(t *testing.T) {
 	}
 }
 
-// persistYamlLogin handles different token formats
-func TestPersistYamlLogin_DifferentTokenFormats(t *testing.T) {
+// persistLogin handles different token formats
+func TestPersistLogin_DifferentTokenFormats(t *testing.T) {
 	testTokens := []string{
 		"eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9...",
 		"simple-token",
@@ -193,27 +210,28 @@ func TestPersistYamlLogin_DifferentTokenFormats(t *testing.T) {
 	for _, token := range testTokens {
 		t.Run("token format", func(t *testing.T) {
 			_ = withTempConfigDir(t)
+			store := swapCredentialResolver(t)
 			cmd, _, _ := newBufferedCmd()
 
-			if err := persistYamlLogin(cmd, token); err != nil {
-				t.Fatalf("persistYamlLogin failed for token %q: %v", token, err)
+			if err := persistLogin(cmd, token); err != nil {
+				t.Fatalf("persistLogin failed for token %q: %v", token, err)
 			}
 
-			if got := readYamlAPIKey(t); got != token {
+			if got := store.Store[credentialstore.CredentialAPIKey]; got != token {
 				t.Errorf("token mismatch for %q: got %q", token, got)
 			}
 		})
 	}
 }
 
-// persistYamlLogin prints success message to stdout
-func TestPersistYamlLogin_PrintsSuccessMessage(t *testing.T) {
+// persistLogin prints success message to stdout
+func TestPersistLogin_PrintsSuccessMessage(t *testing.T) {
 	_ = withTempConfigDir(t)
 	cmd, out, _ := newBufferedCmd()
 	refreshToken := "test-token-456"
 
-	if err := persistYamlLogin(cmd, refreshToken); err != nil {
-		t.Fatalf("persistYamlLogin failed: %v", err)
+	if err := persistLogin(cmd, refreshToken); err != nil {
+		t.Fatalf("persistLogin failed: %v", err)
 	}
 
 	output := out.String()