Force OS keyring for encrypted provider (#741)

Enforces the use of OS keyring for encrypted provider (#741)
Signed-off-by: lujunsan <[email protected]>
Co-authored-by: Don Browne <[email protected]>

Author: lujunsan

pkg/secrets/factory.go

@@ -143,10 +143,37 @@ func validateNoneProvider(result *SetupResult) *SetupResult {
 	return result
 }
 
+// ErrKeyringNotAvailable is returned when the OS keyring is not available for the encrypted provider.
+var ErrKeyringNotAvailable = errors.New("OS keyring is not available. " +
+	"The encrypted provider requires an OS keyring to securely store passwords. " +
+	"Please use a different secrets provider (e.g., 'none' for API/server environments) " +
+	"or ensure your system has a keyring service available")
+
+// IsKeyringAvailable tests if the OS keyring is available by attempting to set and delete a test value.
+func IsKeyringAvailable() bool {
+	testKey := "toolhive-keyring-test"
+	testValue := "test"
+
+	// Try to set a test value
+	if err := keyring.Set(keyringService, testKey, testValue); err != nil {
+		return false
+	}
+
+	// Clean up the test value
+	_ = keyring.Delete(keyringService, testKey)
+
+	return true
+}
+
 // CreateSecretProvider creates the specified type of secrets provider.
 func CreateSecretProvider(managerType ProviderType) (Provider, error) {
 	switch managerType {
 	case EncryptedType:
+		// Enforce keyring availability for encrypted provider
+		if !IsKeyringAvailable() {
+			return nil, ErrKeyringNotAvailable
+		}
+
 		password, err := GetSecretsPassword()
 		if err != nil {
 			return nil, fmt.Errorf("failed to get secrets password: %w", err)
@@ -168,47 +195,45 @@ func CreateSecretProvider(managerType ProviderType) (Provider, error) {
 }
 
 // GetSecretsPassword returns the password to use for encrypting and decrypting secrets.
-// It will attempt to retrieve it from the environment variable TOOLHIVE_SECRETS_PASSWORD.
-// If the environment variable is not set, it will prompt the user to enter a password.
+// It will attempt to retrieve from the OS keyring.
+// If not available, it will fail with an error.
 func GetSecretsPassword() ([]byte, error) {
-	// First, attempt to load the password from the environment variable.
-	password := []byte(os.Getenv(PasswordEnvVar))
-	if len(password) > 0 {
-		return password, nil
-	}
-
-	// If not present, attempt to load the password from the OS keyring.
+	// Attempt to load the password from the OS keyring.
 	keyringSecret, err := keyring.Get(keyringService, keyringService)
 	if err == nil {
 		return []byte(keyringSecret), nil
 	}
 
 	// We need to determine if the error is due to a lack of keyring on the
 	// system or if the keyring is available but nothing was stored.
-	keyringAvailable := errors.Is(err, keyring.ErrNotFound)
-
-	// We cannot ask for a password in a detached process.
-	// We should never trigger this, but this ensures that if there's a bug
-	// then it's easier to find.
-	if process.IsDetached() {
-		return nil, fmt.Errorf("detached process detected, cannot ask for password")
-	}
+	if errors.Is(err, keyring.ErrNotFound) {
+
+		// Keyring is available but no password stored - this should only happen during setup
+		// We cannot ask for a password in a detached process.
+		// We should never trigger this, but this ensures that if there's a bug
+		// then it's easier to find.
+		if process.IsDetached() {
+			return nil, fmt.Errorf("detached process detected, cannot ask for password")
+		}
 
-	// If the keyring is not available, prompt the user for a password.
-	password, err = readPasswordStdin()
-	if err != nil {
-		return nil, fmt.Errorf("failed to read password: %w", err)
-	}
+		// Prompt for password during setup
+		password, err := readPasswordStdin()
+		if err != nil {
+			return nil, fmt.Errorf("failed to read password: %w", err)
+		}
 
-	// If the keyring is available, we can store the password for future use.
-	if keyringAvailable {
+		// Store the password in the keyring for future use
 		logger.Info("writing password to os keyring")
 		err = keyring.Set(keyringService, keyringService, string(password))
 		if err != nil {
 			return nil, fmt.Errorf("failed to store password in keyring: %w", err)
 		}
+
+		return password, nil
 	}
-	return password, nil
+
+	// Assume any other keyring error means keyring is not available
+	return nil, fmt.Errorf("OS keyring is not available: %w", err)
 }
 
 func readPasswordStdin() ([]byte, error) {