Enhance credential store tests and functionality (AST-178846)

internal/credentialstore/resolver.go

@@ -4,10 +4,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"io/fs"
 	"os"
 	"os/user"
 	"path/filepath"
 	"sync"
+	"time"
 
 	"github.com/checkmarx/ast-cli/internal/configfile"
 	"github.com/checkmarx/ast-cli/internal/logger"
@@ -18,6 +20,8 @@ import (
 const (
 	checkmarxDirName  = ".checkmarx"
 	checkmarxFileName = "checkmarxcli.yaml"
+	// ownerOnlyFilePerm matches how configfile writes the config file: owner-only on Unix.
+	ownerOnlyFilePerm = 0o600
 )
 
 // Resolver resolves credential values across explicit, env, keyring and config-file layers.
@@ -93,7 +97,11 @@ func (r *Resolver) Store(ctx context.Context, credentialName, value string) erro
 	if r.policy == PolicyDisabled {
 		return configfile.SetKey(r.filePath, viperKeyFor(credentialName), value)
 	}
-	return r.store.Set(ctx, credentialName, value)
+	err := r.store.Set(ctx, credentialName, value)
+	if err == nil {
+		touchConfigFile(r.filePath)
+	}
+	return err
 }
 
 // Clear removes a credential following the policy, mirroring Store. A missing
@@ -113,7 +121,34 @@ func (r *Resolver) Clear(ctx context.Context, credentialName string) error {
 		}
 		return configfile.RemoveKey(r.filePath, viperKeyFor(credentialName))
 	}
-	return r.store.Delete(ctx, credentialName)
+	err := r.store.Delete(ctx, credentialName)
+	if err == nil {
+		touchConfigFile(r.filePath)
+	}
+	return err
+}
+
+// touchConfigFile bumps the config file's mtime so consumers watching it (cx-agentic-ai's hooks
+// key their caches off it) still see keyring-backed credential changes. Best-effort, never fatal.
+func touchConfigFile(path string) {
+	if path == "" {
+		return
+	}
+	now := time.Now()
+	err := os.Chtimes(path, now, now)
+	if err == nil {
+		return
+	}
+	if !errors.Is(err, fs.ErrNotExist) {
+		logger.PrintfIfVerbose("credentialstore: could not touch config file mtime: %v", err)
+		return
+	}
+	file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY, ownerOnlyFilePerm)
+	if err != nil {
+		logger.PrintfIfVerbose("credentialstore: could not create config file to touch mtime: %v", err)
+		return
+	}
+	_ = file.Close()
 }
 
 // StoresInConfigFile reports whether this policy persists credentials in the

internal/credentialstore/resolver_write_test.go

@@ -2,9 +2,11 @@ package credentialstore
 
 import (
 	"context"
+	"errors"
 	"os"
 	"path/filepath"
 	"testing"
+	"time"
 
 	"github.com/checkmarx/ast-cli/internal/configfile"
 	"github.com/stretchr/testify/assert"
@@ -47,9 +49,9 @@ func TestClearDisabledRemovesYAMLAndReportsMissing(t *testing.T) {
 	assert.Empty(t, value)
 }
 
-// PolicyRequired round-trips through the keyring store only; the YAML layer
-// is neither read nor written.
-func TestStoreRequiredRoundTripIgnoresYAML(t *testing.T) {
+// PolicyRequired round-trips through the keyring store only; Store touches the YAML file
+// but never writes the credential value into it.
+func TestStoreRequiredRoundTripKeepsValueOutOfYAML(t *testing.T) {
 	keyring.MockInit()
 	t.Cleanup(keyring.MockInit)
 	store := NewCredentialStore(CanonicalConfigPath(t.TempDir()))
@@ -58,9 +60,9 @@ func TestStoreRequiredRoundTripIgnoresYAML(t *testing.T) {
 
 	assert.NoError(t, resolver.Store(ctx, CredentialAPIKey, "required-value"))
 
-	if _, err := os.Stat(resolver.filePath); !os.IsNotExist(err) {
-		t.Fatalf("required mode must not create the YAML file")
-	}
+	data, err := os.ReadFile(resolver.filePath)
+	assert.NoError(t, err)
+	assert.NotContains(t, string(data), "required-value")
 	value, err := resolver.Resolve(ctx, CredentialAPIKey)
 	assert.NoError(t, err)
 	assert.Equal(t, "required-value", value)
@@ -142,6 +144,70 @@ func TestClearDisabledUnreadableConfigPropagatesError(t *testing.T) {
 	assert.NotErrorIs(t, err, ErrNotFound)
 }
 
+// A keyring-backed Store/Clear must still bump the config file's mtime, since the credential
+// value itself never lands in the file for mtime-watching consumers to see.
+func TestStoreAndClearAutoTouchConfigFileMtime(t *testing.T) {
+	keyring.MockInit()
+	t.Cleanup(keyring.MockInit)
+	yamlPath := filepath.Join(t.TempDir(), "checkmarxcli.yaml")
+	resolver := NewResolver(yamlPath, PolicyAuto, nil)
+	ctx := context.Background()
+
+	assert.NoError(t, resolver.Store(ctx, CredentialAPIKey, "v"))
+	assert.FileExists(t, yamlPath)
+
+	// Backdate rather than sleep: filesystems with coarse mtime granularity would
+	// otherwise report both writes at the same instant.
+	past := time.Now().Add(-time.Hour)
+	assert.NoError(t, os.Chtimes(yamlPath, past, past))
+	assert.NoError(t, resolver.Clear(ctx, CredentialAPIKey))
+
+	info, err := os.Stat(yamlPath)
+	assert.NoError(t, err)
+	assert.True(t, info.ModTime().After(past))
+}
+
+// A failed keyring write must leave no trace: no config file, hence no mtime bump
+// telling consumers a credential changed.
+func TestStoreFailureDoesNotTouchConfigFile(t *testing.T) {
+	store := newFakeStore()
+	store.setErr = errors.New("keyring write failed")
+	yamlPath := filepath.Join(t.TempDir(), "checkmarxcli.yaml")
+	resolver := NewResolver(yamlPath, PolicyAuto, store)
+
+	assert.Error(t, resolver.Store(context.Background(), CredentialAPIKey, "v"))
+	assert.NoFileExists(t, yamlPath)
+}
+
+// Unusable paths must be swallowed: a best-effort touch never fails a credential write,
+// and it never leaves a stray file behind.
+func TestTouchConfigFileToleratesUnusablePaths(t *testing.T) {
+	missingParent := filepath.Join(t.TempDir(), "no-such-dir", "checkmarxcli.yaml")
+	nonDirParent := filepath.Join(existingFile(t), "checkmarxcli.yaml")
+
+	for name, path := range map[string]string{
+		"empty path":       "",
+		"missing parent":   missingParent,
+		"parent is a file": nonDirParent,
+		"path is a dir":    t.TempDir(),
+	} {
+		t.Run(name, func(t *testing.T) {
+			touchConfigFile(path)
+		})
+	}
+
+	assert.NoFileExists(t, missingParent)
+	assert.NoFileExists(t, nonDirParent)
+}
+
+// existingFile returns a regular file's path, for cases needing a non-directory parent.
+func existingFile(t *testing.T) string {
+	t.Helper()
+	path := filepath.Join(t.TempDir(), "not-a-dir")
+	assert.NoError(t, os.WriteFile(path, []byte("x"), 0o600))
+	return path
+}
+
 // Auto mode surfaces a config-file read failure instead of masking it as
 // not-found once the keyring layer also misses.
 func TestResolveAutoConfigFileReadErrorSurfaces(t *testing.T) {