From 8c79db73d788a44b223c9e0d217192c2b1ab85a2 Mon Sep 17 00:00:00 2001 From: thibaud-leclere Date: Tue, 14 Apr 2026 09:04:40 +0200 Subject: [PATCH] feat: adopt rc3 credential resolution flow --- README.md | 6 ++ go.mod | 2 +- go.sum | 4 +- internal/cli/app.go | 150 ++++++++++++++++++++++++++++++++++--- internal/cli/app_test.go | 156 +++++++++++++++++++++++++++++++++++++++ internal/cli/doctor.go | 73 +++++++++++------- 6 files changed, 352 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 14cd589..9980797 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,12 @@ Le profil actif est résolu dans cet ordre : 3. `current_profile` dans `config.json` 4. `default` +Les credentials IMAP sont résolus ensuite via le résolveur multi-sources du framework (RC3) : + +1. `host` : `EMAIL_MCP_HOST` puis `config.json` +2. `username` : `EMAIL_MCP_USERNAME` puis `config.json` +3. `password` : `EMAIL_MCP_PASSWORD` puis secret wallet `imap-password/` + ### Configurer un profil ```sh diff --git a/go.mod b/go.mod index 041c674..f129a21 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module email-mcp go 1.25.0 require ( - gitea.lclr.dev/AI/mcp-framework v1.2.0-rc2 + gitea.lclr.dev/AI/mcp-framework v1.2.0-rc3 github.com/emersion/go-imap/v2 v2.0.0-beta.8 github.com/emersion/go-message v0.18.2 github.com/godbus/dbus/v5 v5.2.2 diff --git a/go.sum b/go.sum index c2399c5..0d0d322 100644 --- a/go.sum +++ b/go.sum @@ -1,5 +1,5 @@ -gitea.lclr.dev/AI/mcp-framework v1.2.0-rc2 h1:nzeW1JkGPV/+Hhhtdy7EWWeDQNjt36qMeVQjJYmGCQE= -gitea.lclr.dev/AI/mcp-framework v1.2.0-rc2/go.mod h1:kUVMrL3/UBYgjOsW7sJCs3V0pO0qoJJMpIpueoTsoA4= +gitea.lclr.dev/AI/mcp-framework v1.2.0-rc3 h1:pbG3eFQbBBVZDlNMA1MY3ZYocVGiZT0z95dHOUbSJYQ= +gitea.lclr.dev/AI/mcp-framework v1.2.0-rc3/go.mod h1:kUVMrL3/UBYgjOsW7sJCs3V0pO0qoJJMpIpueoTsoA4= github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 h1:/vQbFIOMbk2FiG/kXiLl8BRyzTWDw7gX/Hz7Dd5eDMs= github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4/go.mod h1:hN7oaIRCjzsZ2dE+yG5k+rsdt3qcwykqK6HVGcKwsw4= github.com/99designs/keyring v1.2.2 h1:pZd3neh/EmUzWONb35LxQfvuY7kiSXAq3HQd97+XBn0= diff --git a/internal/cli/app.go b/internal/cli/app.go index 1e10017..2a91b50 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -24,6 +24,9 @@ import ( const ( binaryName = "email-mcp" defaultProfileEnv = "EMAIL_MCP_PROFILE" + hostEnv = "EMAIL_MCP_HOST" + usernameEnv = "EMAIL_MCP_USERNAME" + passwordEnv = "EMAIL_MCP_PASSWORD" binaryDescription = "Local MCP server to read an IMAP mailbox." ) @@ -384,28 +387,30 @@ func (a *App) loadCredential(profileFlag string) (secretstore.Credential, error) } profileName := frameworkcli.ResolveProfileName(profileFlag, os.Getenv(defaultProfileEnv), cfg.CurrentProfile) - profile, ok := cfg.Profiles[profileName] - if !ok { - return secretstore.Credential{}, fmt.Errorf("%w: profile %q", mcpserver.ErrCredentialsNotConfigured, profileName) - } + profile := cfg.Profiles[profileName] secrets, err := a.openSecretStore() if err != nil { return secretstore.Credential{}, err } - password, _, err := loadStoredPassword(secrets, profileName) + resolution, err := resolveCredentialFields(profile, secrets, credentialFieldSpecs(profileName)) if err != nil { - if errors.Is(err, frameworksecretstore.ErrNotFound) { - return secretstore.Credential{}, fmt.Errorf("%w: profile %q", mcpserver.ErrCredentialsNotConfigured, profileName) + var missingErr *frameworkcli.MissingRequiredValuesError + if errors.As(err, &missingErr) { + return secretstore.Credential{}, fmt.Errorf( + "%w: profile %q is incomplete (missing: %s)", + mcpserver.ErrCredentialsNotConfigured, + profileName, + strings.Join(missingErr.Fields, ", "), + ) } return secretstore.Credential{}, err } - cred := secretstore.Credential{ - Host: profile.Host, - Username: profile.Username, - Password: password, + cred, err := credentialFromResolution(resolution) + if err != nil { + return secretstore.Credential{}, err } if err := cred.Validate(); err != nil { return secretstore.Credential{}, fmt.Errorf("%w: profile %q is incomplete", mcpserver.ErrCredentialsNotConfigured, profileName) @@ -414,6 +419,129 @@ func (a *App) loadCredential(profileFlag string) (secretstore.Credential, error) return cred, nil } +func credentialFieldSpecs(profileName string) []frameworkcli.FieldSpec { + return []frameworkcli.FieldSpec{ + { + Name: "host", + Required: true, + Sources: []frameworkcli.ValueSource{ + frameworkcli.SourceEnv, + frameworkcli.SourceConfig, + }, + EnvKey: hostEnv, + ConfigKey: "host", + }, + { + Name: "username", + Required: true, + Sources: []frameworkcli.ValueSource{ + frameworkcli.SourceEnv, + frameworkcli.SourceConfig, + }, + EnvKey: usernameEnv, + ConfigKey: "username", + }, + passwordFieldSpec(profileName), + } +} + +func profileFieldSpecs() []frameworkcli.FieldSpec { + return []frameworkcli.FieldSpec{ + { + Name: "host", + Required: true, + Sources: []frameworkcli.ValueSource{ + frameworkcli.SourceEnv, + frameworkcli.SourceConfig, + }, + EnvKey: hostEnv, + ConfigKey: "host", + }, + { + Name: "username", + Required: true, + Sources: []frameworkcli.ValueSource{ + frameworkcli.SourceEnv, + frameworkcli.SourceConfig, + }, + EnvKey: usernameEnv, + ConfigKey: "username", + }, + } +} + +func passwordFieldSpec(profileName string) frameworkcli.FieldSpec { + return frameworkcli.FieldSpec{ + Name: "password", + Required: true, + Sources: []frameworkcli.ValueSource{ + frameworkcli.SourceEnv, + frameworkcli.SourceSecret, + }, + EnvKey: passwordEnv, + SecretKey: passwordSecretName(profileName), + } +} + +func resolveCredentialFields(profile ProfileConfig, store secretStore, fields []frameworkcli.FieldSpec) (frameworkcli.Resolution, error) { + configValues := map[string]string{ + "host": profile.Host, + "username": profile.Username, + } + + return frameworkcli.ResolveFields(frameworkcli.ResolveOptions{ + Fields: fields, + Lookup: func(source frameworkcli.ValueSource, key string) (string, bool, error) { + switch source { + case frameworkcli.SourceEnv: + value, ok := os.LookupEnv(strings.TrimSpace(key)) + return value, ok, nil + case frameworkcli.SourceConfig: + value, ok := configValues[strings.TrimSpace(key)] + return value, ok, nil + case frameworkcli.SourceSecret: + if store == nil { + return "", false, nil + } + + value, err := store.GetSecret(strings.TrimSpace(key)) + if err != nil { + if errors.Is(err, frameworksecretstore.ErrNotFound) { + return "", false, nil + } + return "", false, err + } + return value, true, nil + default: + return "", false, nil + } + }, + }) +} + +func credentialFromResolution(resolution frameworkcli.Resolution) (secretstore.Credential, error) { + host, ok := resolution.Get("host") + if !ok { + return secretstore.Credential{}, fmt.Errorf("resolve credential: host field is missing from resolution") + } + + username, ok := resolution.Get("username") + if !ok { + return secretstore.Credential{}, fmt.Errorf("resolve credential: username field is missing from resolution") + } + + password, ok := resolution.Get("password") + if !ok { + return secretstore.Credential{}, fmt.Errorf("resolve credential: password field is missing from resolution") + } + + return secretstore.Credential{ + Host: host.Value, + Username: username.Value, + Password: password.Value, + }, nil +} + func loadStoredPassword(store secretStore, profileName string) (string, bool, error) { password, err := store.GetSecret(passwordSecretName(profileName)) if err != nil { diff --git a/internal/cli/app_test.go b/internal/cli/app_test.go index d7b2952..612a75c 100644 --- a/internal/cli/app_test.go +++ b/internal/cli/app_test.go @@ -372,6 +372,107 @@ func TestAppRunMCPDelegatesResolvedCredentialToRunner(t *testing.T) { } } +func TestAppRunMCPPrefersEnvironmentCredentialValues(t *testing.T) { + t.Setenv(hostEnv, "imap.env.example.com") + t.Setenv(usernameEnv, "alice-env") + t.Setenv(passwordEnv, "secret-env") + + cfgStore := &configStoreStub{ + cfg: frameworkconfig.FileConfig[ProfileConfig]{ + Version: frameworkconfig.CurrentVersion, + CurrentProfile: "work", + Profiles: map[string]ProfileConfig{ + "work": { + Host: "imap.config.example.com", + Username: "alice-config", + }, + }, + }, + } + secrets := &secretStoreStub{ + values: map[string]string{ + "imap-password/work": "secret-wallet", + }, + } + runner := &runnerStub{} + var gotCredential secretstore.Credential + + app := NewAppWithDependencies( + nil, + cfgStore, + func() (secretStore, error) { return secrets, nil }, + func() mcpserver.MailService { return wireMailServiceStub{} }, + func(cred secretstore.Credential, _ mcpserver.MailService, _ io.Reader, _ io.Writer, _ io.Writer) MCPRunner { + gotCredential = cred + return runner + }, + nil, + nil, + nil, + nil, + &bytes.Buffer{}, + "dev", + ) + + if err := app.Run([]string{"mcp"}); err != nil { + t.Fatalf("mcp returned error: %v", err) + } + if !runner.called { + t.Fatal("expected runner to be called") + } + + want := secretstore.Credential{ + Host: "imap.env.example.com", + Username: "alice-env", + Password: "secret-env", + } + if gotCredential != want { + t.Fatalf("credential = %#v, want %#v", gotCredential, want) + } +} + +func TestAppRunMCPUsesEnvironmentCredentialWithoutSavedProfile(t *testing.T) { + t.Setenv(hostEnv, "imap.env.example.com") + t.Setenv(usernameEnv, "alice-env") + t.Setenv(passwordEnv, "secret-env") + + cfgStore := &configStoreStub{ + cfg: frameworkconfig.FileConfig[ProfileConfig]{ + Version: frameworkconfig.CurrentVersion, + }, + } + secrets := &secretStoreStub{} + runner := &runnerStub{} + var gotCredential secretstore.Credential + + app := NewAppWithDependencies( + nil, + cfgStore, + func() (secretStore, error) { return secrets, nil }, + func() mcpserver.MailService { return wireMailServiceStub{} }, + func(cred secretstore.Credential, _ mcpserver.MailService, _ io.Reader, _ io.Writer, _ io.Writer) MCPRunner { + gotCredential = cred + return runner + }, + nil, + nil, + nil, + nil, + &bytes.Buffer{}, + "dev", + ) + + if err := app.Run([]string{"mcp"}); err != nil { + t.Fatalf("mcp returned error: %v", err) + } + if !runner.called { + t.Fatal("expected runner to be called") + } + if gotCredential.Host != "imap.env.example.com" || gotCredential.Username != "alice-env" || gotCredential.Password != "secret-env" { + t.Fatalf("unexpected credential %#v", gotCredential) + } +} + func TestAppRunUpdateLoadsManifestNearExecutable(t *testing.T) { tempDir := t.TempDir() executablePath := filepath.Join(tempDir, "email-mcp") @@ -563,6 +664,61 @@ latest_release_url = "https://example.com/releases/latest" } } +func TestAppRunDoctorAcceptsPasswordFromEnvironment(t *testing.T) { + tempHome := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", tempHome) + t.Setenv("HOME", tempHome) + t.Setenv(passwordEnv, "env-secret") + + store := frameworkconfig.NewStore[ProfileConfig](binaryName) + configPath, err := store.ConfigPath() + if err != nil { + t.Fatalf("ConfigPath returned error: %v", err) + } + if err := store.Save(configPath, frameworkconfig.FileConfig[ProfileConfig]{ + Version: frameworkconfig.CurrentVersion, + CurrentProfile: "work", + Profiles: map[string]ProfileConfig{ + "work": { + Host: "imap.example.com", + Username: "alice", + }, + }, + }); err != nil { + t.Fatalf("Save returned error: %v", err) + } + + manifestDir := t.TempDir() + if err := os.WriteFile(filepath.Join(manifestDir, "mcp.toml"), []byte(` +[update] +latest_release_url = "https://example.com/releases/latest" +`), 0o600); err != nil { + t.Fatalf("WriteFile returned error: %v", err) + } + + output := &bytes.Buffer{} + app := NewAppWithDependencies( + nil, + store, + func() (secretStore, error) { return &secretStoreStub{}, nil }, + func() mcpserver.MailService { return &doctorMailServiceStub{} }, + nil, + nil, + func() (string, error) { return filepath.Join(manifestDir, "email-mcp"), nil }, + nil, + output, + &bytes.Buffer{}, + "dev", + ) + + if err := app.Run([]string{"doctor"}); err != nil { + t.Fatalf("doctor returned error: %v", err) + } + if !strings.Contains(output.String(), "[OK] password: password is provided via environment") { + t.Fatalf("unexpected output: %q", output.String()) + } +} + func TestAppRunReturnsClearErrorsWhenDependenciesMissing(t *testing.T) { app := NewAppWithDependencies(nil, nil, nil, nil, nil, nil, nil, nil, nil, &bytes.Buffer{}, "dev") diff --git a/internal/cli/doctor.go b/internal/cli/doctor.go index 1c3850c..e5d21e9 100644 --- a/internal/cli/doctor.go +++ b/internal/cli/doctor.go @@ -2,6 +2,7 @@ package cli import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -89,37 +90,39 @@ func (a *App) doctorProfileCheck(profileFlag string) frameworkcli.DoctorCheck { } profileName := frameworkcli.ResolveProfileName(profileFlag, os.Getenv(defaultProfileEnv), cfg.CurrentProfile) - profile, ok := cfg.Profiles[profileName] - if !ok { + resolution, err := resolveCredentialFields(cfg.Profiles[profileName], nil, profileFieldSpecs()) + if err != nil { + var missingErr *frameworkcli.MissingRequiredValuesError + if errors.As(err, &missingErr) { + return frameworkcli.DoctorResult{ + Name: "profile", + Status: frameworkcli.DoctorStatusFail, + Summary: "resolved profile is incomplete", + Detail: fmt.Sprintf("profile %q: missing %s", profileName, strings.Join(missingErr.Fields, ", ")), + } + } + return frameworkcli.DoctorResult{ Name: "profile", Status: frameworkcli.DoctorStatusFail, - Summary: "resolved profile is missing", - Detail: fmt.Sprintf("profile %q", profileName), + Summary: "cannot resolve profile values", + Detail: err.Error(), } } - var issues []string - if strings.TrimSpace(profile.Host) == "" { - issues = append(issues, "host is empty") - } - if strings.TrimSpace(profile.Username) == "" { - issues = append(issues, "username is empty") - } - if len(issues) > 0 { - return frameworkcli.DoctorResult{ - Name: "profile", - Status: frameworkcli.DoctorStatusFail, - Summary: "resolved profile is incomplete", - Detail: fmt.Sprintf("profile %q: %s", profileName, strings.Join(issues, "; ")), - } - } + host, _ := resolution.Get("host") + username, _ := resolution.Get("username") return frameworkcli.DoctorResult{ Name: "profile", Status: frameworkcli.DoctorStatusOK, Summary: "resolved profile is complete", - Detail: fmt.Sprintf("profile %q", profileName), + Detail: fmt.Sprintf( + "profile %q (host: %s, username: %s)", + profileName, + host.Source, + username.Source, + ), } } } @@ -137,8 +140,26 @@ func (a *App) doctorPasswordCheck(profileFlag string) frameworkcli.DoctorCheck { } } - _, hasPassword, err := loadStoredPassword(store, profileName) + resolution, err := resolveCredentialFields( + ProfileConfig{}, + store, + []frameworkcli.FieldSpec{passwordFieldSpec(profileName)}, + ) if err != nil { + var missingErr *frameworkcli.MissingRequiredValuesError + if errors.As(err, &missingErr) { + return frameworkcli.DoctorResult{ + Name: "password", + Status: frameworkcli.DoctorStatusFail, + Summary: "stored password is missing", + Detail: fmt.Sprintf( + "set %q or secret %q", + passwordEnv, + passwordSecretName(profileName), + ), + } + } + return frameworkcli.DoctorResult{ Name: "password", Status: frameworkcli.DoctorStatusFail, @@ -146,12 +167,14 @@ func (a *App) doctorPasswordCheck(profileFlag string) frameworkcli.DoctorCheck { Detail: err.Error(), } } - if !hasPassword { + + password, _ := resolution.Get("password") + if password.Source == frameworkcli.SourceEnv { return frameworkcli.DoctorResult{ Name: "password", - Status: frameworkcli.DoctorStatusFail, - Summary: "stored password is missing", - Detail: fmt.Sprintf("secret %q", passwordSecretName(profileName)), + Status: frameworkcli.DoctorStatusOK, + Summary: "password is provided via environment", + Detail: fmt.Sprintf("variable %q", passwordEnv), } }