Merge pull request 'Supporter des secrets structurés et des politiques de backend' (#11) from issue-4-structured-secrets into main

Reviewed-on: https://gitea.lclr.dev/AI/mcp-framework/pulls/11

AGENTS.md

@@ -0,0 +1,60 @@
+# AGENTS.md
+
+Ces instructions s'appliquent à tout le dépôt `mcp-framework`.
+
+## Forge
+
+Le dépôt est hébergé sur Gitea.
+
+Pour les actions distantes liées au dépôt, utiliser `tea` plutôt qu'un autre CLI.
+Exemples : lire une issue, s'assigner une issue, créer une branche de travail, ouvrir une PR, commenter une PR.
+
+## Issues
+
+Quand on commence à travailler sur une issue, il faut se l'assigner immédiatement.
+
+Si le travail correspond directement à une issue existante :
+
+1. lire l'issue
+2. se l'assigner
+3. créer une branche dédiée
+4. implémenter et valider localement
+5. pousser la branche
+6. ouvrir une PR liée à l'issue
+
+## Branches
+
+Préférer une branche de travail dédiée par issue ou sujet.
+
+Nommer la branche de manière explicite, par exemple :
+
+- `issue-4-structured-secrets`
+- `issue-8-update-drivers`
+- `docs-readme-installation`
+
+Éviter de développer directement sur `main` quand le changement mérite une PR ou une validation fonctionnelle.
+
+## Pull Requests
+
+Par défaut, quand un changement doit être validé via un vrai use case, une intégration avec un autre dépôt, ou une revue fonctionnelle, ouvrir une PR plutôt que pousser directement sur `main`.
+
+Le corps de PR doit en général contenir :
+
+- un résumé clair de ce qui a été implémenté
+- la validation locale déjà effectuée
+- une proposition de test manuel ou d'intégration si pertinente
+- le lien explicite avec l'issue, idéalement via `Closes #<numéro>`
+
+Si le changement a un impact possible sur un dépôt consommateur comme `graylog-mcp` ou `email-mcp`, ajouter dans la PR ou dans un commentaire de PR un plan de validation ou d'adaptation pour ce dépôt.
+
+## Validation
+
+Avant d'ouvrir ou de mettre à jour une PR :
+
+- exécuter les tests locaux pertinents
+- signaler clairement ce qui a été validé
+- signaler explicitement ce qui n'a pas pu être testé
+
+## Commits
+
+Conserver des messages de commit au format conventional commits, conformément aux règles globales.

README.md

@@ -115,7 +115,14 @@ Notes :
 
 ## Secrets
 
-Le package `secretstore` s'appuie sur le wallet natif du système :
+Le package `secretstore` supporte plusieurs politiques de backend :
+
+- `auto` : comportement par défaut, utilise un backend keyring disponible et peut retomber sur l'environnement si `LookupEnv` est fourni
+- `kwallet-only` : impose KWallet et retourne une erreur explicite si KWallet n'est pas disponible
+- `keyring-any` : impose l'utilisation d'un backend keyring disponible
+- `env-only` : lecture seule depuis les variables d'environnement
+
+Backends keyring typiques :
 
 - macOS : Keychain
 - Linux : Secret Service ou KWallet selon l'environnement
@@ -125,7 +132,8 @@ Exemple :
 
 ```go
 store, err := secretstore.Open(secretstore.Options{
-	ServiceName: "my-mcp",
+	ServiceName:   "my-mcp",
+	BackendPolicy: secretstore.BackendAuto,
 })
 if err != nil {
 	return err
@@ -146,6 +154,42 @@ default:
 }
 ```
 
+Pour imposer KWallet sur Linux :
+
+```go
+store, err := secretstore.Open(secretstore.Options{
+	ServiceName:   "email-mcp",
+	BackendPolicy: secretstore.BackendKWalletOnly,
+})
+```
+
+Pour stocker un secret structuré en JSON :
+
+```go
+type Credentials struct {
+	Host     string `json:"host"`
+	Username string `json:"username"`
+	Password string `json:"password"`
+}
+
+err = secretstore.SetJSON(store, "imap-credentials", "IMAP credentials", Credentials{
+	Host:     "imap.example.com",
+	Username: "alice",
+	Password: token,
+})
+if err != nil {
+	return err
+}
+
+creds, err := secretstore.GetJSON[Credentials](store, "imap-credentials")
+if err != nil {
+	return err
+}
+```
+
+En mode `env-only`, `GetSecret("API_TOKEN")` lit la variable d'environnement `API_TOKEN`.
+Les opérations d'écriture et de suppression retournent `secretstore.ErrReadOnly`.
+
 ## Helpers CLI
 
 `cli` fournit des helpers simples pour les assistants interactifs :

secretstore/store.go

@@ -1,18 +1,36 @@
 package secretstore
 
 import (
+	"encoding/json"
 	"errors"
 	"fmt"
+	"os"
 	"runtime"
+	"slices"
 	"strings"
 
 	"github.com/99designs/keyring"
 )
 
 var ErrNotFound = errors.New("secret not found")
+var ErrBackendUnavailable = errors.New("secret backend unavailable")
+var ErrReadOnly = errors.New("secret backend is read-only")
+
+type BackendPolicy string
+
+const (
+	BackendAuto        BackendPolicy = "auto"
+	BackendKWalletOnly BackendPolicy = "kwallet-only"
+	BackendKeyringAny  BackendPolicy = "keyring-any"
+	BackendEnvOnly     BackendPolicy = "env-only"
+)
 
 type Options struct {
-	ServiceName string
+	ServiceName   string
+	BackendPolicy BackendPolicy
+	LookupEnv     func(string) (string, bool)
+	KWalletAppID  string
+	KWalletFolder string
 }
 
 type Store interface {
@@ -21,28 +39,91 @@ type Store interface {
 	DeleteSecret(name string) error
 }
 
+type BackendUnavailableError struct {
+	Policy    BackendPolicy
+	Required  string
+	Available []string
+}
+
+func (e *BackendUnavailableError) Error() string {
+	if len(e.Available) == 0 {
+		return fmt.Sprintf("secret backend policy %q requires %s, but no compatible backend is available", e.Policy, e.Required)
+	}
+
+	return fmt.Sprintf(
+		"secret backend policy %q requires %s, but only [%s] are available",
+		e.Policy,
+		e.Required,
+		strings.Join(e.Available, ", "),
+	)
+}
+
+func (e *BackendUnavailableError) Unwrap() error {
+	return ErrBackendUnavailable
+}
+
 type keyringStore struct {
 	ring        keyring.Keyring
 	serviceName string
 }
 
+type envStore struct {
+	lookupEnv func(string) (string, bool)
+}
+
+var (
+	openKeyring            = keyring.Open
+	availableKeyringPolicy = keyring.AvailableBackends
+)
+
 func Open(options Options) (Store, error) {
+	policy := options.BackendPolicy
+	if policy == "" {
+		policy = BackendAuto
+	}
+
+	switch policy {
+	case BackendAuto, BackendKWalletOnly, BackendKeyringAny, BackendEnvOnly:
+	default:
+		return nil, fmt.Errorf("invalid secret backend policy %q", policy)
+	}
+
+	if policy == BackendEnvOnly {
+		return newEnvStore(options), nil
+	}
+
 	serviceName := strings.TrimSpace(options.ServiceName)
 	if serviceName == "" {
 		return nil, errors.New("service name must not be empty")
 	}
 
-	ring, err := keyring.Open(keyring.Config{
-		ServiceName: serviceName,
-	})
+	available := availableKeyringPolicy()
+	allowed, err := allowedBackends(policy, available)
 	if err != nil {
-		return nil, fmt.Errorf("open OS wallet backend %q for service %q: %w", BackendName(), serviceName, err)
+		if errors.Is(err, ErrBackendUnavailable) && policy == BackendAuto && options.LookupEnv != nil {
+			return newEnvStore(options), nil
+		}
+		return nil, err
 	}
 
-	return &keyringStore{
-		ring:        ring,
-		serviceName: serviceName,
-	}, nil
+	ring, err := openKeyring(keyring.Config{
+		ServiceName:     serviceName,
+		AllowedBackends: allowed,
+		KWalletAppID:    strings.TrimSpace(options.KWalletAppID),
+		KWalletFolder:   strings.TrimSpace(options.KWalletFolder),
+	})
+	if err == nil {
+		return &keyringStore{
+			ring:        ring,
+			serviceName: serviceName,
+		}, nil
+	}
+
+	if policy == BackendAuto && options.LookupEnv != nil {
+		return newEnvStore(options), nil
+	}
+
+	return nil, fmt.Errorf("open secret backend for service %q with policy %q: %w", serviceName, policy, err)
 }
 
 func BackendName() string {
@@ -58,6 +139,38 @@ func BackendName() string {
 	}
 }
 
+func SetJSON[T any](store Store, name, label string, value T) error {
+	data, err := json.Marshal(value)
+	if err != nil {
+		return fmt.Errorf("encode structured secret %q as JSON: %w", name, err)
+	}
+
+	return store.SetSecret(name, label, string(data))
+}
+
+func GetJSON[T any](store Store, name string) (T, error) {
+	var value T
+	if err := GetJSONInto(store, name, &value); err != nil {
+		var zero T
+		return zero, err
+	}
+
+	return value, nil
+}
+
+func GetJSONInto(store Store, name string, target any) error {
+	secret, err := store.GetSecret(name)
+	if err != nil {
+		return err
+	}
+
+	if err := json.Unmarshal([]byte(secret), target); err != nil {
+		return fmt.Errorf("decode structured secret %q from JSON: %w", name, err)
+	}
+
+	return nil
+}
+
 func (s *keyringStore) SetSecret(name, label, secret string) error {
 	if err := s.ring.Set(keyring.Item{
 		Key:   name,
@@ -88,3 +201,62 @@ func (s *keyringStore) DeleteSecret(name string) error {
 	}
 	return nil
 }
+
+func (s *envStore) SetSecret(name, label, secret string) error {
+	return fmt.Errorf("save secret %q in environment backend: %w", name, ErrReadOnly)
+}
+
+func (s *envStore) GetSecret(name string) (string, error) {
+	value, ok := s.lookupEnv(name)
+	if !ok {
+		return "", ErrNotFound
+	}
+
+	return value, nil
+}
+
+func (s *envStore) DeleteSecret(name string) error {
+	return fmt.Errorf("delete secret %q from environment backend: %w", name, ErrReadOnly)
+}
+
+func newEnvStore(options Options) Store {
+	lookupEnv := options.LookupEnv
+	if lookupEnv == nil {
+		lookupEnv = os.LookupEnv
+	}
+
+	return &envStore{lookupEnv: lookupEnv}
+}
+
+func allowedBackends(policy BackendPolicy, available []keyring.BackendType) ([]keyring.BackendType, error) {
+	switch policy {
+	case BackendAuto, BackendKeyringAny:
+		if len(available) == 0 {
+			return nil, &BackendUnavailableError{
+				Policy:    policy,
+				Required:  "any keyring backend",
+				Available: nil,
+			}
+		}
+		return slices.Clone(available), nil
+	case BackendKWalletOnly:
+		if !slices.Contains(available, keyring.KWalletBackend) {
+			return nil, &BackendUnavailableError{
+				Policy:    policy,
+				Required:  string(keyring.KWalletBackend),
+				Available: backendNames(available),
+			}
+		}
+		return []keyring.BackendType{keyring.KWalletBackend}, nil
+	default:
+		return nil, fmt.Errorf("invalid secret backend policy %q", policy)
+	}
+}
+
+func backendNames(backends []keyring.BackendType) []string {
+	names := make([]string, 0, len(backends))
+	for _, backend := range backends {
+		names = append(names, string(backend))
+	}
+	return names
+}

secretstore/store_test.go

@@ -0,0 +1,247 @@
+package secretstore
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/99designs/keyring"
+)
+
+type stubKeyring struct {
+	items map[string]keyring.Item
+}
+
+func (s *stubKeyring) Get(key string) (keyring.Item, error) {
+	item, ok := s.items[key]
+	if !ok {
+		return keyring.Item{}, keyring.ErrKeyNotFound
+	}
+
+	return item, nil
+}
+
+func (s *stubKeyring) GetMetadata(key string) (keyring.Metadata, error) {
+	item, err := s.Get(key)
+	if err != nil {
+		return keyring.Metadata{}, err
+	}
+
+	return keyring.Metadata{Item: &item}, nil
+}
+
+func (s *stubKeyring) Set(item keyring.Item) error {
+	if s.items == nil {
+		s.items = map[string]keyring.Item{}
+	}
+
+	s.items[item.Key] = item
+	return nil
+}
+
+func (s *stubKeyring) Remove(key string) error {
+	delete(s.items, key)
+	return nil
+}
+
+func (s *stubKeyring) Keys() ([]string, error) {
+	keys := make([]string, 0, len(s.items))
+	for key := range s.items {
+		keys = append(keys, key)
+	}
+
+	return keys, nil
+}
+
+func withKeyringHooks(
+	t *testing.T,
+	available []keyring.BackendType,
+	opener func(cfg keyring.Config) (keyring.Keyring, error),
+) {
+	t.Helper()
+
+	prevAvailable := availableKeyringPolicy
+	prevOpen := openKeyring
+	availableKeyringPolicy = func() []keyring.BackendType {
+		return available
+	}
+	openKeyring = opener
+
+	t.Cleanup(func() {
+		availableKeyringPolicy = prevAvailable
+		openKeyring = prevOpen
+	})
+}
+
+func TestOpenRejectsInvalidPolicy(t *testing.T) {
+	_, err := Open(Options{
+		ServiceName:   "mcp-framework-test",
+		BackendPolicy: BackendPolicy("invalid"),
+	})
+	if err == nil {
+		t.Fatal("expected error")
+	}
+}
+
+func TestOpenAutoUsesAvailableKeyringBackends(t *testing.T) {
+	var gotAllowed []keyring.BackendType
+	ring := &stubKeyring{}
+
+	withKeyringHooks(t, []keyring.BackendType{
+		keyring.SecretServiceBackend,
+		keyring.KWalletBackend,
+	}, func(cfg keyring.Config) (keyring.Keyring, error) {
+		gotAllowed = append([]keyring.BackendType(nil), cfg.AllowedBackends...)
+		return ring, nil
+	})
+
+	store, err := Open(Options{
+		ServiceName:   "mcp-framework-test",
+		BackendPolicy: BackendAuto,
+	})
+	if err != nil {
+		t.Fatalf("Open returned error: %v", err)
+	}
+
+	if err := store.SetSecret("token", "API token", "secret-value"); err != nil {
+		t.Fatalf("SetSecret returned error: %v", err)
+	}
+
+	value, err := store.GetSecret("token")
+	if err != nil {
+		t.Fatalf("GetSecret returned error: %v", err)
+	}
+	if value != "secret-value" {
+		t.Fatalf("GetSecret = %q, want secret-value", value)
+	}
+
+	if len(gotAllowed) != 2 {
+		t.Fatalf("allowed backends = %v, want two entries", gotAllowed)
+	}
+}
+
+func TestOpenAutoFallsBackToEnvironmentWhenNoKeyringBackendIsAvailable(t *testing.T) {
+	withKeyringHooks(t, nil, func(cfg keyring.Config) (keyring.Keyring, error) {
+		t.Fatal("unexpected keyring open call")
+		return nil, nil
+	})
+
+	store, err := Open(Options{
+		ServiceName:   "mcp-framework-test",
+		BackendPolicy: BackendAuto,
+		LookupEnv: func(name string) (string, bool) {
+			if name == "EMAIL_CREDENTIALS" {
+				return `{"username":"alice"}`, true
+			}
+			return "", false
+		},
+	})
+	if err != nil {
+		t.Fatalf("Open returned error: %v", err)
+	}
+
+	value, err := store.GetSecret("EMAIL_CREDENTIALS")
+	if err != nil {
+		t.Fatalf("GetSecret returned error: %v", err)
+	}
+	if value != `{"username":"alice"}` {
+		t.Fatalf("GetSecret = %q", value)
+	}
+}
+
+func TestOpenKeyringAnyReturnsExplicitUnavailableError(t *testing.T) {
+	withKeyringHooks(t, []keyring.BackendType{keyring.SecretServiceBackend}, func(cfg keyring.Config) (keyring.Keyring, error) {
+		t.Fatal("unexpected keyring open call")
+		return nil, nil
+	})
+
+	_, err := Open(Options{
+		ServiceName:   "mcp-framework-test",
+		BackendPolicy: BackendKWalletOnly,
+	})
+	if err == nil {
+		t.Fatal("expected error")
+	}
+
+	var backendErr *BackendUnavailableError
+	if !errors.As(err, &backendErr) {
+		t.Fatalf("error = %v, want BackendUnavailableError", err)
+	}
+	if !errors.Is(err, ErrBackendUnavailable) {
+		t.Fatalf("error = %v, want ErrBackendUnavailable", err)
+	}
+	if backendErr.Required != "kwallet" {
+		t.Fatalf("required backend = %q, want kwallet", backendErr.Required)
+	}
+}
+
+func TestOpenEnvOnlyIsReadOnlyAndUsesLookupEnv(t *testing.T) {
+	store, err := Open(Options{
+		BackendPolicy: BackendEnvOnly,
+		LookupEnv: func(name string) (string, bool) {
+			if name == "API_TOKEN" {
+				return "from-env", true
+			}
+			return "", false
+		},
+	})
+	if err != nil {
+		t.Fatalf("Open returned error: %v", err)
+	}
+
+	value, err := store.GetSecret("API_TOKEN")
+	if err != nil {
+		t.Fatalf("GetSecret returned error: %v", err)
+	}
+	if value != "from-env" {
+		t.Fatalf("GetSecret = %q, want from-env", value)
+	}
+
+	err = store.SetSecret("API_TOKEN", "API token", "new-value")
+	if !errors.Is(err, ErrReadOnly) {
+		t.Fatalf("SetSecret error = %v, want ErrReadOnly", err)
+	}
+
+	err = store.DeleteSecret("API_TOKEN")
+	if !errors.Is(err, ErrReadOnly) {
+		t.Fatalf("DeleteSecret error = %v, want ErrReadOnly", err)
+	}
+}
+
+func TestJSONHelpersRoundTrip(t *testing.T) {
+	ring := &stubKeyring{}
+	withKeyringHooks(t, []keyring.BackendType{keyring.SecretServiceBackend}, func(cfg keyring.Config) (keyring.Keyring, error) {
+		return ring, nil
+	})
+
+	store, err := Open(Options{
+		ServiceName: "mcp-framework-test",
+	})
+	if err != nil {
+		t.Fatalf("Open returned error: %v", err)
+	}
+
+	type credentials struct {
+		Host     string `json:"host"`
+		Username string `json:"username"`
+		Password string `json:"password"`
+	}
+
+	input := credentials{
+		Host:     "imap.example.com",
+		Username: "alice",
+		Password: "s3cr3t",
+	}
+
+	if err := SetJSON(store, "imap-credentials", "IMAP credentials", input); err != nil {
+		t.Fatalf("SetJSON returned error: %v", err)
+	}
+
+	output, err := GetJSON[credentials](store, "imap-credentials")
+	if err != nil {
+		t.Fatalf("GetJSON returned error: %v", err)
+	}
+
+	if output != input {
+		t.Fatalf("GetJSON = %#v, want %#v", output, input)
+	}
+}