From 7d159bfdbd6a4ed7cc7a3ff27f436646e7600e2a Mon Sep 17 00:00:00 2001 From: thibaud-lclr Date: Mon, 20 Apr 2026 10:56:15 +0200 Subject: [PATCH] feat: add runtime secretstore diagnostics and setup helpers --- cli/doctor.go | 53 +++++++-- cli/doctor_test.go | 82 ++++++++++++++ cli/setup_secret.go | 76 +++++++++++++ cli/setup_secret_test.go | 116 ++++++++++++++++++++ docs/cli-helpers.md | 27 +++-- docs/packages.md | 2 +- docs/secrets.md | 76 +++++++++++++ scaffold/scaffold.go | 57 +++------- scaffold/scaffold_test.go | 2 + secretstore/bitwarden.go | 63 ++++++++++- secretstore/bitwarden_test.go | 48 +++++++++ secretstore/manifest_open.go | 54 +++++++--- secretstore/runtime.go | 196 ++++++++++++++++++++++++++++++++++ secretstore/runtime_test.go | 155 +++++++++++++++++++++++++++ secretstore/store.go | 1 + 15 files changed, 934 insertions(+), 74 deletions(-) create mode 100644 cli/setup_secret.go create mode 100644 cli/setup_secret_test.go create mode 100644 secretstore/runtime.go create mode 100644 secretstore/runtime_test.go diff --git a/cli/doctor.go b/cli/doctor.go index ec859e7..293806e 100644 --- a/cli/doctor.go +++ b/cli/doctor.go @@ -49,19 +49,23 @@ type DoctorSecret struct { type DoctorManifestValidator func(manifest.File, string) []string type DoctorOptions struct { - ConfigCheck DoctorCheck - SecretStoreCheck DoctorCheck - RequiredSecrets []DoctorSecret - SecretStoreFactory func() (secretstore.Store, error) - ManifestDir string - ManifestValidator DoctorManifestValidator - ManifestCheck DoctorCheck - ConnectivityCheck DoctorCheck - ExtraChecks []DoctorCheck + ConfigCheck DoctorCheck + SecretStoreCheck DoctorCheck + SecretBackendPolicy secretstore.BackendPolicy + RequiredSecrets []DoctorSecret + SecretStoreFactory func() (secretstore.Store, error) + ManifestDir string + ManifestValidator DoctorManifestValidator + ManifestCheck DoctorCheck + ConnectivityCheck DoctorCheck + BitwardenOptions BitwardenDoctorOptions + DisableAutoBitwardenCheck bool + ExtraChecks []DoctorCheck } type BitwardenDoctorOptions struct { Command string + Shell string LookupEnv func(string) (string, bool) } @@ -87,6 +91,9 @@ func RunDoctor(ctx context.Context, options DoctorOptions) DoctorReport { if options.ConnectivityCheck != nil { checks = append(checks, options.ConnectivityCheck) } + if shouldAutoIncludeBitwardenCheck(options) { + checks = append(checks, BitwardenReadyCheck(options.BitwardenOptions)) + } checks = append(checks, options.ExtraChecks...) results := make([]DoctorResult, 0, len(checks)) @@ -231,6 +238,7 @@ func BitwardenReadyCheck(options BitwardenDoctorOptions) DoctorCheck { return func(context.Context) DoctorResult { err := checkBitwardenReady(secretstore.Options{ BitwardenCommand: strings.TrimSpace(options.Command), + Shell: strings.TrimSpace(options.Shell), LookupEnv: options.LookupEnv, }) if err == nil { @@ -274,6 +282,33 @@ func BitwardenReadyCheck(options BitwardenDoctorOptions) DoctorCheck { } } +func shouldAutoIncludeBitwardenCheck(options DoctorOptions) bool { + if options.DisableAutoBitwardenCheck { + return false + } + + if options.SecretBackendPolicy == secretstore.BackendBitwardenCLI { + return true + } + + if options.SecretStoreFactory == nil { + return false + } + + store, err := options.SecretStoreFactory() + if err == nil { + return secretstore.EffectiveBackendPolicy(store) == secretstore.BackendBitwardenCLI + } + + if errors.Is(err, secretstore.ErrBWNotLoggedIn) || + errors.Is(err, secretstore.ErrBWLocked) || + errors.Is(err, secretstore.ErrBWUnavailable) { + return true + } + + return strings.Contains(strings.ToLower(strings.TrimSpace(err.Error())), "bitwarden") +} + func RequiredSecretsCheck(factory func() (secretstore.Store, error), required []DoctorSecret) DoctorCheck { return func(context.Context) DoctorResult { store, err := factory() diff --git a/cli/doctor_test.go b/cli/doctor_test.go index 698cd74..bbdc15e 100644 --- a/cli/doctor_test.go +++ b/cli/doctor_test.go @@ -253,6 +253,88 @@ func TestBitwardenReadyCheckMapsTypedErrorsToActionableDiagnostics(t *testing.T) }) } +func TestRunDoctorAutoInjectsBitwardenCheckForBitwardenPolicy(t *testing.T) { + prev := checkBitwardenReady + t.Cleanup(func() { + checkBitwardenReady = prev + }) + + lookupCalled := false + checkBitwardenReady = func(options secretstore.Options) error { + if options.Shell != "fish" { + t.Fatalf("Shell = %q, want fish", options.Shell) + } + if options.BitwardenCommand != "bw" { + t.Fatalf("BitwardenCommand = %q, want bw", options.BitwardenCommand) + } + if options.LookupEnv == nil { + t.Fatal("LookupEnv should be forwarded") + } + _, _ = options.LookupEnv("BW_SESSION") + return fmt.Errorf("%w: run unlock", secretstore.ErrBWLocked) + } + + report := RunDoctor(context.Background(), DoctorOptions{ + SecretBackendPolicy: secretstore.BackendBitwardenCLI, + BitwardenOptions: BitwardenDoctorOptions{ + Command: "bw", + Shell: "fish", + LookupEnv: func(name string) (string, bool) { + lookupCalled = true + return "", false + }, + }, + ManifestCheck: func(context.Context) DoctorResult { + return DoctorResult{Name: "manifest", Status: DoctorStatusOK, Summary: "manifest ok"} + }, + }) + + if !lookupCalled { + t.Fatal("LookupEnv should be used by auto bitwarden check") + } + + var found *DoctorResult + for i := range report.Results { + if report.Results[i].Name == "bitwarden" { + found = &report.Results[i] + break + } + } + if found == nil { + t.Fatalf("report results = %#v, want auto bitwarden check", report.Results) + } + if found.Status != DoctorStatusFail { + t.Fatalf("bitwarden status = %q, want fail", found.Status) + } +} + +func TestRunDoctorCanDisableAutoBitwardenCheck(t *testing.T) { + prev := checkBitwardenReady + t.Cleanup(func() { + checkBitwardenReady = prev + }) + + checkBitwardenReady = func(options secretstore.Options) error { + t.Fatal("checkBitwardenReady should not be called when auto check is disabled") + return nil + } + + report := RunDoctor(context.Background(), DoctorOptions{ + DisableAutoBitwardenCheck: true, + SecretBackendPolicy: secretstore.BackendBitwardenCLI, + ManifestCheck: func(context.Context) DoctorResult { + return DoctorResult{Name: "manifest", Status: DoctorStatusOK, Summary: "manifest ok"} + }, + }) + + if len(report.Results) != 1 { + t.Fatalf("result count = %d, want 1", len(report.Results)) + } + if report.Results[0].Name != "manifest" { + t.Fatalf("result name = %q, want manifest", report.Results[0].Name) + } +} + func TestRequiredResolvedFieldsCheckReportsSources(t *testing.T) { check := RequiredResolvedFieldsCheck(ResolveOptions{ Fields: []FieldSpec{ diff --git a/cli/setup_secret.go b/cli/setup_secret.go new file mode 100644 index 0000000..dceaacd --- /dev/null +++ b/cli/setup_secret.go @@ -0,0 +1,76 @@ +package cli + +import ( + "errors" + "fmt" + "strings" + + "gitea.lclr.dev/AI/mcp-framework/secretstore" +) + +type SetupSecretWriteOptions struct { + Store secretstore.Store + SecretName string + SecretLabel string + TokenEnv string + Value SetupValue +} + +func WriteSetupSecretVerified(options SetupSecretWriteOptions) error { + if options.Store == nil { + return errors.New("secret store must not be nil") + } + + secretName := strings.TrimSpace(options.SecretName) + if secretName == "" { + return errors.New("secret name must not be empty") + } + + secretLabel := strings.TrimSpace(options.SecretLabel) + if secretLabel == "" { + secretLabel = secretName + } + + if options.Value.KeptStoredSecret { + return verifyStoredSetupSecret(options.Store, secretName, options.TokenEnv) + } + if !options.Value.Set { + return nil + } + + if err := secretstore.SetSecretVerified(options.Store, secretName, secretLabel, options.Value.String); err != nil { + if errors.Is(err, secretstore.ErrReadOnly) { + tokenEnv := strings.TrimSpace(options.TokenEnv) + if tokenEnv != "" { + return fmt.Errorf("secret store is read-only, export %s and retry setup: %w", tokenEnv, err) + } + } + return fmt.Errorf("save secret %q during setup: %w", secretName, err) + } + + return nil +} + +func verifyStoredSetupSecret(store secretstore.Store, secretName, tokenEnv string) error { + secret, err := store.GetSecret(secretName) + if err != nil { + if errors.Is(err, secretstore.ErrNotFound) { + tokenEnv = strings.TrimSpace(tokenEnv) + if tokenEnv != "" { + return fmt.Errorf( + "secret %q is not readable after setup, export %s and retry: %w", + secretName, + tokenEnv, + err, + ) + } + } + return fmt.Errorf("verify secret %q after setup: %w", secretName, err) + } + + if strings.TrimSpace(secret) == "" { + return fmt.Errorf("secret %q is empty after setup", secretName) + } + + return nil +} diff --git a/cli/setup_secret_test.go b/cli/setup_secret_test.go new file mode 100644 index 0000000..d426459 --- /dev/null +++ b/cli/setup_secret_test.go @@ -0,0 +1,116 @@ +package cli + +import ( + "errors" + "strings" + "testing" + + "gitea.lclr.dev/AI/mcp-framework/secretstore" +) + +func TestWriteSetupSecretVerifiedPersistsAndConfirmsReadability(t *testing.T) { + store := &setupSecretStore{secrets: map[string]string{}} + + err := WriteSetupSecretVerified(SetupSecretWriteOptions{ + Store: store, + SecretName: "api-token", + SecretLabel: "API token", + Value: SetupValue{ + Type: SetupFieldSecret, + String: "secret-v1", + Set: true, + }, + }) + if err != nil { + t.Fatalf("WriteSetupSecretVerified returned error: %v", err) + } + + if got := store.secrets["api-token"]; got != "secret-v1" { + t.Fatalf("stored secret = %q, want secret-v1", got) + } +} + +func TestWriteSetupSecretVerifiedReturnsContextForReadOnlyStores(t *testing.T) { + store := &setupSecretStore{ + secrets: map[string]string{}, + setErr: secretstore.ErrReadOnly, + } + + err := WriteSetupSecretVerified(SetupSecretWriteOptions{ + Store: store, + SecretName: "api-token", + TokenEnv: "GRAYLOG_MCP_API_TOKEN", + Value: SetupValue{ + Type: SetupFieldSecret, + String: "secret-v1", + Set: true, + }, + }) + if err == nil { + t.Fatal("expected error") + } + if !errors.Is(err, secretstore.ErrReadOnly) { + t.Fatalf("error = %v, want ErrReadOnly", err) + } + if !strings.Contains(err.Error(), "GRAYLOG_MCP_API_TOKEN") { + t.Fatalf("error = %v, want token env remediation", err) + } +} + +func TestWriteSetupSecretVerifiedValidatesKeptStoredSecret(t *testing.T) { + store := &setupSecretStore{ + secrets: map[string]string{}, + getErr: secretstore.ErrNotFound, + } + + err := WriteSetupSecretVerified(SetupSecretWriteOptions{ + Store: store, + SecretName: "api-token", + TokenEnv: "GRAYLOG_MCP_API_TOKEN", + Value: SetupValue{ + Type: SetupFieldSecret, + String: "stored-token", + Set: true, + KeptStoredSecret: true, + }, + }) + if err == nil { + t.Fatal("expected error") + } + if !errors.Is(err, secretstore.ErrNotFound) { + t.Fatalf("error = %v, want ErrNotFound", err) + } + if !strings.Contains(err.Error(), "GRAYLOG_MCP_API_TOKEN") { + t.Fatalf("error = %v, want token env remediation", err) + } +} + +type setupSecretStore struct { + secrets map[string]string + setErr error + getErr error +} + +func (s *setupSecretStore) SetSecret(name, label, secret string) error { + if s.setErr != nil { + return s.setErr + } + s.secrets[name] = secret + return nil +} + +func (s *setupSecretStore) GetSecret(name string) (string, error) { + if s.getErr != nil { + return "", s.getErr + } + value, ok := s.secrets[name] + if !ok { + return "", secretstore.ErrNotFound + } + return value, nil +} + +func (s *setupSecretStore) DeleteSecret(name string) error { + delete(s.secrets, name) + return nil +} diff --git a/docs/cli-helpers.md b/docs/cli-helpers.md index 69d97b3..7f59080 100644 --- a/docs/cli-helpers.md +++ b/docs/cli-helpers.md @@ -73,6 +73,20 @@ _ = enabled _ = scopes ``` +Pour persister un secret de setup avec write+read-back et messages homogènes : + +```go +if err := cli.WriteSetupSecretVerified(cli.SetupSecretWriteOptions{ + Store: store, + SecretName: "api-token", + SecretLabel: "API token", + TokenEnv: "MY_MCP_API_TOKEN", + Value: apiToken, +}); err != nil { + return err +} +``` + Chaque champ peut déclarer ses propres hooks de validation (`Validate`, `ValidateBool`, `ValidateList`). Les validations sont appliquées de manière cohérente en TTY et en stdin non interactif. @@ -123,6 +137,10 @@ report := cli.RunDoctor(context.Background(), cli.DoctorOptions{ ServiceName: "my-mcp", }) }), + SecretBackendPolicy: secretstore.BackendBitwardenCLI, + BitwardenOptions: cli.BitwardenDoctorOptions{ + LookupEnv: os.LookupEnv, + }, RequiredSecrets: []cli.DoctorSecret{ {Name: "api-token", Label: "API token"}, }, @@ -158,15 +176,12 @@ if report.HasFailures() { } ``` -Pour une policy `bitwarden-cli`, tu peux ajouter un check dédié avec remédiations : +Quand `SecretBackendPolicy` vaut `bitwarden-cli`, le check Bitwarden est ajouté automatiquement. +Pour le désactiver explicitement : ```go report := cli.RunDoctor(context.Background(), cli.DoctorOptions{ - ExtraChecks: []cli.DoctorCheck{ - cli.BitwardenReadyCheck(cli.BitwardenDoctorOptions{ - LookupEnv: os.LookupEnv, - }), - }, + DisableAutoBitwardenCheck: true, }) ``` diff --git a/docs/packages.md b/docs/packages.md index 4c181c2..1e197e2 100644 --- a/docs/packages.md +++ b/docs/packages.md @@ -5,5 +5,5 @@ - `config` : lecture/écriture atomique d'une config JSON versionnée dans `os.UserConfigDir()`. - `manifest` : lecture de `mcp.toml` à la racine du projet, fallback embarqué pour le runtime, conversion vers `update.ReleaseSource` et exposition de métadonnées pour `bootstrap`/scaffolding. - `scaffold` : génération d'un squelette de projet MCP (arborescence, `main.go`, `mcp.toml`, `install.sh` wizard, wiring de base et README de démarrage). -- `secretstore` : lecture/écriture de secrets dans le wallet natif, avec helper runtime `OpenFromManifest`. +- `secretstore` : lecture/écriture de secrets dans le wallet natif, avec helpers runtime `OpenFromManifest`, `DescribeRuntime`, `PreflightFromManifest` et formatage homogène via `FormatBackendStatus`. - `update` : téléchargement et remplacement du binaire courant depuis un endpoint de release. diff --git a/docs/secrets.md b/docs/secrets.md index 9d9e683..24046d1 100644 --- a/docs/secrets.md +++ b/docs/secrets.md @@ -131,5 +131,81 @@ effective := secretstore.EffectiveBackendPolicy(store) fmt.Println("backend effectif:", effective) // bitwarden-cli, env-only, keyring-any... ``` +Pour obtenir en un seul appel une description runtime (source manifeste, policy déclarée/effective, disponibilité) : + +```go +desc, err := secretstore.DescribeRuntime(secretstore.DescribeRuntimeOptions{ + ServiceName: "my-mcp", + LookupEnv: os.LookupEnv, +}) +if err != nil { + return err +} + +fmt.Println(secretstore.FormatBackendStatus(desc)) +// declared=... effective=... display=... ready=... source=... +``` + +Pour un préflight réutilisable dans `setup`, `config show` et `config test` : + +```go +report, err := secretstore.PreflightFromManifest(secretstore.PreflightOptions{ + ServiceName: "my-mcp", + LookupEnv: os.LookupEnv, +}) +if err != nil { + return err +} + +fmt.Println(report.Status) // ready | fail +fmt.Println(report.Summary) // message court +fmt.Println(report.Remediation) // action recommandée +``` + +## Debug Bitwarden en 60 secondes + +1. Vérifier l'état de session : + +```bash +bw status +``` + +2. Déverrouiller le vault et exporter `BW_SESSION` : + +- Bash/Zsh : + +```bash +export BW_SESSION="$(bw unlock --raw)" +``` + +- Fish : + +```fish +set -x BW_SESSION (bw unlock --raw) +``` + +- PowerShell : + +```powershell +$env:BW_SESSION = (bw unlock --raw) +``` + +3. Vérifier lecture/écriture rapide : + +```go +if err := store.SetSecret("debug-token", "Debug token", "ok"); err != nil { + return err +} +_, err := store.GetSecret("debug-token") +return err +``` + +4. Interpréter les erreurs typées : + +- `secretstore.ErrBWNotLoggedIn` : `bw login` requis. +- `secretstore.ErrBWLocked` : vault verrouillé ou `BW_SESSION` absent. +- `secretstore.ErrBWUnavailable` : CLI/réseau indisponible. +- `secretstore.ErrBackendUnavailable` : policy non satisfiable dans le contexte courant. + 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`. diff --git a/scaffold/scaffold.go b/scaffold/scaffold.go index a94b97c..1856f3c 100644 --- a/scaffold/scaffold.go +++ b/scaffold/scaffold.go @@ -1731,37 +1731,19 @@ func (r Runtime) runSetup(_ context.Context, inv bootstrap.Invocation) error { return err } - if !tokenValue.KeptStoredSecret { - store, err := r.openSecretStore() - if err != nil { - return err - } - - if err := secretstore.SetSecretVerified(store, r.SecretName, "API token", tokenValue.String); err != nil { - if errors.Is(err, secretstore.ErrReadOnly) { - return fmt.Errorf( - "secret store is read-only, export %s and retry setup", - r.TokenEnv, - ) - } else { - return err - } - } - } - - verifiedToken, err := r.readToken() + store, err := r.openSecretStore() if err != nil { - if errors.Is(err, secretstore.ErrNotFound) { - return fmt.Errorf( - "secret %q is not readable after setup, export %s and retry", - r.SecretName, - r.TokenEnv, - ) - } return err } - if strings.TrimSpace(verifiedToken) == "" { - return fmt.Errorf("secret %q is empty after setup", r.SecretName) + + if err := cli.WriteSetupSecretVerified(cli.SetupSecretWriteOptions{ + Store: store, + SecretName: r.SecretName, + SecretLabel: "API token", + TokenEnv: r.TokenEnv, + Value: tokenValue, + }); err != nil { + return err } _, err = fmt.Fprintf(stdout, "Configuration saved for profile %q. Secret readability confirmed.\n", profileName) @@ -1835,15 +1817,16 @@ func (r Runtime) runConfigTest(ctx context.Context, inv bootstrap.Invocation) er } report := cli.RunDoctor(ctx, cli.DoctorOptions{ - ConfigCheck: cli.NewConfigCheck(r.ConfigStore), - SecretStoreCheck: cli.SecretStoreAvailabilityCheck(r.openSecretStore), + ConfigCheck: cli.NewConfigCheck(r.ConfigStore), + SecretStoreCheck: cli.SecretStoreAvailabilityCheck(r.openSecretStore), + SecretBackendPolicy: r.activeBackendPolicy(), RequiredSecrets: []cli.DoctorSecret{ {Name: r.SecretName, Label: "API token"}, }, SecretStoreFactory: r.openSecretStore, ManifestCheck: r.manifestDoctorCheck(), - ExtraChecks: []cli.DoctorCheck{ - r.bitwardenDoctorCheck(), + BitwardenOptions: cli.BitwardenDoctorOptions{ + LookupEnv: os.LookupEnv, }, }) @@ -1947,16 +1930,6 @@ func (r Runtime) manifestDoctorCheck() cli.DoctorCheck { } } } - -func (r Runtime) bitwardenDoctorCheck() cli.DoctorCheck { - if r.activeBackendPolicy() != secretstore.BackendBitwardenCLI { - return nil - } - - return cli.BitwardenReadyCheck(cli.BitwardenDoctorOptions{ - LookupEnv: os.LookupEnv, - }) -} ` const manifestTemplate = `binary_name = "{{.BinaryName}}" diff --git a/scaffold/scaffold_test.go b/scaffold/scaffold_test.go index 67e3aef..5880090 100644 --- a/scaffold/scaffold_test.go +++ b/scaffold/scaffold_test.go @@ -75,6 +75,8 @@ func TestGenerateCreatesRecommendedSkeleton(t *testing.T) { `var embeddedManifest = `, "ManifestSource", "ManifestCheck: r.manifestDoctorCheck()", + "SecretBackendPolicy: r.activeBackendPolicy()", + "cli.WriteSetupSecretVerified", } { if !strings.Contains(string(appGo), snippet) { t.Fatalf("app.go missing snippet %q", snippet) diff --git a/secretstore/bitwarden.go b/secretstore/bitwarden.go index 2a29cee..ffcc0f7 100644 --- a/secretstore/bitwarden.go +++ b/secretstore/bitwarden.go @@ -7,6 +7,8 @@ import ( "fmt" "os" "os/exec" + "path/filepath" + "runtime" "strings" ) @@ -68,6 +70,7 @@ func newBitwardenStore(options Options, policy BackendPolicy, serviceName string if err := EnsureBitwardenReady(Options{ BitwardenCommand: command, LookupEnv: options.LookupEnv, + Shell: options.Shell, }); err != nil { return nil, fmt.Errorf( "secret backend policy %q cannot use bitwarden CLI command %q right now: %w", @@ -85,6 +88,7 @@ func EnsureBitwardenReady(options Options) error { if command == "" { command = defaultBitwardenCommand } + unlockCommand := bitwardenUnlockRemediation(command, options.Shell) lookupEnv := options.LookupEnv if lookupEnv == nil { @@ -111,18 +115,18 @@ func EnsureBitwardenReady(options Options) error { return fmt.Errorf("%w: run `bw login` then retry", ErrBWNotLoggedIn) case "locked": return fmt.Errorf( - "%w: run `export %s=\"$(bw unlock --raw)\"` then retry", + "%w: run `%s` then retry", ErrBWLocked, - bitwardenSessionEnvName, + unlockCommand, ) case "unlocked": session, ok := lookupEnv(bitwardenSessionEnvName) if !ok || strings.TrimSpace(session) == "" { return fmt.Errorf( - "%w: environment variable %q is missing; run `export %s=\"$(bw unlock --raw)\"` then retry", + "%w: environment variable %q is missing; run `%s` then retry", ErrBWLocked, bitwardenSessionEnvName, - bitwardenSessionEnvName, + unlockCommand, ) } return nil @@ -135,6 +139,57 @@ func EnsureBitwardenReady(options Options) error { } } +func bitwardenUnlockRemediation(command, shellHint string) string { + unlockCommand := fmt.Sprintf("%s unlock --raw", strings.TrimSpace(command)) + + switch detectShellFlavor(shellHint) { + case "fish": + return fmt.Sprintf("set -x %s (%s)", bitwardenSessionEnvName, unlockCommand) + case "powershell": + return fmt.Sprintf("$env:%s = (%s)", bitwardenSessionEnvName, unlockCommand) + case "cmd": + return fmt.Sprintf( + "for /f \"usebackq delims=\" %%i in (`%s`) do set %s=%%i", + unlockCommand, + bitwardenSessionEnvName, + ) + default: + return fmt.Sprintf("export %s=\"$(%s)\"", bitwardenSessionEnvName, unlockCommand) + } +} + +func detectShellFlavor(shellHint string) string { + raw := strings.TrimSpace(shellHint) + if raw == "" { + raw = strings.TrimSpace(os.Getenv("SHELL")) + } + if raw == "" { + raw = strings.TrimSpace(os.Getenv("COMSPEC")) + } + if raw == "" && runtime.GOOS == "windows" { + return "powershell" + } + + lower := strings.ToLower(strings.TrimSpace(raw)) + base := strings.ToLower(filepath.Base(lower)) + + switch { + case strings.Contains(lower, "powershell"), + strings.Contains(lower, "pwsh"), + base == "powershell", + base == "powershell.exe", + base == "pwsh", + base == "pwsh.exe": + return "powershell" + case strings.Contains(lower, "fish"), base == "fish": + return "fish" + case strings.Contains(lower, "cmd.exe"), base == "cmd", base == "cmd.exe": + return "cmd" + default: + return "posix" + } +} + func (s *bitwardenStore) SetSecret(name, label, secret string) error { secretName := s.scopedName(name) item, err := s.findItem(secretName, name) diff --git a/secretstore/bitwarden_test.go b/secretstore/bitwarden_test.go index 9994fb5..e12a662 100644 --- a/secretstore/bitwarden_test.go +++ b/secretstore/bitwarden_test.go @@ -140,6 +140,54 @@ func TestEnsureBitwardenReadyAcceptsUnlockedSession(t *testing.T) { } } +func TestEnsureBitwardenReadyAdaptsUnlockRemediationToShell(t *testing.T) { + t.Run("fish", func(t *testing.T) { + withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) { + if len(args) == 1 && args[0] == "status" { + return []byte(`{"status":"locked"}`), nil + } + return nil, fmt.Errorf("unexpected args: %v", args) + }) + + err := EnsureBitwardenReady(Options{BitwardenCommand: "bw", Shell: "fish"}) + if err == nil { + t.Fatal("expected error") + } + if !errors.Is(err, ErrBWLocked) { + t.Fatalf("error = %v, want ErrBWLocked", err) + } + if !strings.Contains(err.Error(), "set -x BW_SESSION (bw unlock --raw)") { + t.Fatalf("error = %v, want fish unlock remediation", err) + } + }) + + t.Run("powershell", func(t *testing.T) { + withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) { + if len(args) == 1 && args[0] == "status" { + return []byte(`{"status":"unlocked"}`), nil + } + return nil, fmt.Errorf("unexpected args: %v", args) + }) + + err := EnsureBitwardenReady(Options{ + BitwardenCommand: "bw", + Shell: "powershell", + LookupEnv: func(name string) (string, bool) { + return "", false + }, + }) + if err == nil { + t.Fatal("expected error") + } + if !errors.Is(err, ErrBWLocked) { + t.Fatalf("error = %v, want ErrBWLocked", err) + } + if !strings.Contains(err.Error(), "$env:BW_SESSION = (bw unlock --raw)") { + t.Fatalf("error = %v, want powershell unlock remediation", err) + } + }) +} + func TestBitwardenStoreSetGetDeleteSecret(t *testing.T) { withBitwardenSession(t) fakeCLI := newFakeBitwardenCLI("bw") diff --git a/secretstore/manifest_open.go b/secretstore/manifest_open.go index 71e0390..9c95653 100644 --- a/secretstore/manifest_open.go +++ b/secretstore/manifest_open.go @@ -19,26 +19,43 @@ type OpenFromManifestOptions struct { LookupEnv func(string) (string, bool) KWalletAppID string KWalletFolder string + BitwardenCommand string + Shell string ManifestLoader ManifestLoader ExecutableResolver ExecutableResolver } func OpenFromManifest(options OpenFromManifestOptions) (Store, error) { - policy, err := resolveManifestBackendPolicy(options) + manifestPolicy, err := resolveManifestPolicy(options) if err != nil { return nil, err } return Open(Options{ - ServiceName: options.ServiceName, - BackendPolicy: policy, - LookupEnv: options.LookupEnv, - KWalletAppID: options.KWalletAppID, - KWalletFolder: options.KWalletFolder, + ServiceName: options.ServiceName, + BackendPolicy: manifestPolicy.Policy, + LookupEnv: options.LookupEnv, + KWalletAppID: options.KWalletAppID, + KWalletFolder: options.KWalletFolder, + BitwardenCommand: strings.TrimSpace(options.BitwardenCommand), + Shell: strings.TrimSpace(options.Shell), }) } func resolveManifestBackendPolicy(options OpenFromManifestOptions) (BackendPolicy, error) { + resolution, err := resolveManifestPolicy(options) + if err != nil { + return "", err + } + return resolution.Policy, nil +} + +type manifestPolicyResolution struct { + Policy BackendPolicy + Source string +} + +func resolveManifestPolicy(options OpenFromManifestOptions) (manifestPolicyResolution, error) { manifestLoader := options.ManifestLoader if manifestLoader == nil { manifestLoader = manifest.LoadDefault @@ -51,7 +68,7 @@ func resolveManifestBackendPolicy(options OpenFromManifestOptions) (BackendPolic executablePath, err := executableResolver() if err != nil { - return "", fmt.Errorf("resolve executable path for manifest lookup: %w", err) + return manifestPolicyResolution{}, fmt.Errorf("resolve executable path for manifest lookup: %w", err) } startDir := filepath.Dir(strings.TrimSpace(executablePath)) @@ -62,19 +79,32 @@ func resolveManifestBackendPolicy(options OpenFromManifestOptions) (BackendPolic file, manifestPath, err := manifestLoader(startDir) if err != nil { if errors.Is(err, os.ErrNotExist) { - return BackendAuto, nil + return manifestPolicyResolution{ + Policy: BackendAuto, + Source: "", + }, nil } - return "", fmt.Errorf("load runtime manifest from %q: %w", startDir, err) + return manifestPolicyResolution{}, fmt.Errorf("load runtime manifest from %q: %w", startDir, err) } if strings.TrimSpace(file.SecretStore.BackendPolicy) == "" { - return BackendAuto, nil + return manifestPolicyResolution{ + Policy: BackendAuto, + Source: strings.TrimSpace(manifestPath), + }, nil } policy, err := normalizeBackendPolicy(BackendPolicy(file.SecretStore.BackendPolicy)) if err != nil { - return "", fmt.Errorf("invalid secret_store.backend_policy in manifest %q: %w", strings.TrimSpace(manifestPath), err) + return manifestPolicyResolution{}, fmt.Errorf( + "invalid secret_store.backend_policy in manifest %q: %w", + strings.TrimSpace(manifestPath), + err, + ) } - return policy, nil + return manifestPolicyResolution{ + Policy: policy, + Source: strings.TrimSpace(manifestPath), + }, nil } diff --git a/secretstore/runtime.go b/secretstore/runtime.go new file mode 100644 index 0000000..ec724dc --- /dev/null +++ b/secretstore/runtime.go @@ -0,0 +1,196 @@ +package secretstore + +import ( + "errors" + "fmt" + "strings" +) + +const DefaultManifestSource = "default:auto (manifest not found)" + +type DescribeRuntimeOptions struct { + ServiceName string + LookupEnv func(string) (string, bool) + KWalletAppID string + KWalletFolder string + BitwardenCommand string + Shell string + ManifestLoader ManifestLoader + ExecutableResolver ExecutableResolver +} + +type RuntimeDescription struct { + ManifestSource string + DeclaredPolicy BackendPolicy + EffectivePolicy BackendPolicy + DisplayName string + Ready bool + ReadyError error +} + +type PreflightStatus string + +const ( + PreflightStatusReady PreflightStatus = "ready" + PreflightStatusFail PreflightStatus = "fail" +) + +type PreflightOptions = DescribeRuntimeOptions + +type PreflightReport struct { + Status PreflightStatus + Summary string + Remediation string + Runtime RuntimeDescription +} + +func DescribeRuntime(options DescribeRuntimeOptions) (RuntimeDescription, error) { + resolution, err := resolveManifestPolicy(OpenFromManifestOptions{ + ServiceName: options.ServiceName, + LookupEnv: options.LookupEnv, + KWalletAppID: options.KWalletAppID, + KWalletFolder: options.KWalletFolder, + BitwardenCommand: options.BitwardenCommand, + Shell: options.Shell, + ManifestLoader: options.ManifestLoader, + ExecutableResolver: options.ExecutableResolver, + }) + if err != nil { + return RuntimeDescription{}, err + } + + desc := RuntimeDescription{ + ManifestSource: manifestSourceLabel(resolution.Source), + DeclaredPolicy: resolution.Policy, + EffectivePolicy: resolution.Policy, + DisplayName: BackendDisplayName(resolution.Policy), + } + + store, openErr := Open(Options{ + ServiceName: options.ServiceName, + BackendPolicy: resolution.Policy, + LookupEnv: options.LookupEnv, + KWalletAppID: options.KWalletAppID, + KWalletFolder: options.KWalletFolder, + BitwardenCommand: options.BitwardenCommand, + Shell: options.Shell, + }) + if openErr != nil { + desc.Ready = false + desc.ReadyError = openErr + return desc, nil + } + + desc.Ready = true + if effective := EffectiveBackendPolicy(store); strings.TrimSpace(string(effective)) != "" { + desc.EffectivePolicy = effective + desc.DisplayName = BackendDisplayName(effective) + } + + return desc, nil +} + +func PreflightFromManifest(options PreflightOptions) (PreflightReport, error) { + desc, err := DescribeRuntime(options) + if err != nil { + return PreflightReport{}, err + } + + if desc.Ready { + return PreflightReport{ + Status: PreflightStatusReady, + Summary: "secret backend is ready", + Runtime: desc, + }, nil + } + + summary, remediation := summarizePreflightFailure(desc.ReadyError) + return PreflightReport{ + Status: PreflightStatusFail, + Summary: summary, + Remediation: remediation, + Runtime: desc, + }, nil +} + +func BackendDisplayName(policy BackendPolicy) string { + switch policy { + case BackendBitwardenCLI: + return "Bitwarden CLI" + case BackendEnvOnly: + return "Environment variables" + case BackendKWalletOnly: + return "KWallet" + case BackendAuto: + return "automatic backend selection" + case BackendKeyringAny: + return BackendName() + default: + trimmed := strings.TrimSpace(string(policy)) + if trimmed == "" { + return "unknown backend" + } + return trimmed + } +} + +func FormatBackendStatus(desc RuntimeDescription) string { + source := manifestSourceLabel(desc.ManifestSource) + display := strings.TrimSpace(desc.DisplayName) + if display == "" { + display = BackendDisplayName(desc.EffectivePolicy) + } + + effective := desc.EffectivePolicy + if strings.TrimSpace(string(effective)) == "" { + effective = desc.DeclaredPolicy + } + + parts := []string{ + fmt.Sprintf("declared=%s", normalizeStatusPolicy(desc.DeclaredPolicy)), + fmt.Sprintf("effective=%s", normalizeStatusPolicy(effective)), + fmt.Sprintf("display=%s", display), + fmt.Sprintf("ready=%t", desc.Ready), + fmt.Sprintf("source=%s", source), + } + if desc.ReadyError != nil { + parts = append(parts, fmt.Sprintf("error=%s", strings.TrimSpace(desc.ReadyError.Error()))) + } + + return strings.Join(parts, " ") +} + +func summarizePreflightFailure(err error) (string, string) { + if err == nil { + return "secret backend is unavailable", "" + } + + switch { + case errors.Is(err, ErrBWNotLoggedIn): + return "bitwarden login is required", strings.TrimSpace(err.Error()) + case errors.Is(err, ErrBWLocked): + return "bitwarden vault is locked or BW_SESSION is missing", strings.TrimSpace(err.Error()) + case errors.Is(err, ErrBWUnavailable): + return "bitwarden CLI is unavailable", strings.TrimSpace(err.Error()) + case errors.Is(err, ErrBackendUnavailable): + return "secret backend is unavailable", strings.TrimSpace(err.Error()) + default: + return "secret backend preflight failed", strings.TrimSpace(err.Error()) + } +} + +func manifestSourceLabel(source string) string { + trimmed := strings.TrimSpace(source) + if trimmed == "" { + return DefaultManifestSource + } + return trimmed +} + +func normalizeStatusPolicy(policy BackendPolicy) string { + trimmed := strings.TrimSpace(string(policy)) + if trimmed == "" { + return string(BackendAuto) + } + return trimmed +} diff --git a/secretstore/runtime_test.go b/secretstore/runtime_test.go new file mode 100644 index 0000000..3daf05b --- /dev/null +++ b/secretstore/runtime_test.go @@ -0,0 +1,155 @@ +package secretstore + +import ( + "errors" + "path/filepath" + "strings" + "testing" + + "gitea.lclr.dev/AI/mcp-framework/manifest" +) + +func TestDescribeRuntimeReturnsDeclaredAndEffectivePolicies(t *testing.T) { + desc, err := DescribeRuntime(DescribeRuntimeOptions{ + ServiceName: "graylog-mcp", + LookupEnv: func(string) (string, bool) { return "", false }, + ExecutableResolver: func() (string, error) { + return filepath.Join(string(filepath.Separator), "opt", "graylog-mcp", "bin", "graylog-mcp"), nil + }, + ManifestLoader: func(startDir string) (manifest.File, string, error) { + return manifest.File{ + SecretStore: manifest.SecretStore{BackendPolicy: string(BackendEnvOnly)}, + }, filepath.Join(startDir, manifest.DefaultFile), nil + }, + }) + if err != nil { + t.Fatalf("DescribeRuntime returned error: %v", err) + } + + if desc.ManifestSource == "" { + t.Fatal("ManifestSource should not be empty") + } + if desc.DeclaredPolicy != BackendEnvOnly { + t.Fatalf("DeclaredPolicy = %q, want %q", desc.DeclaredPolicy, BackendEnvOnly) + } + if desc.EffectivePolicy != BackendEnvOnly { + t.Fatalf("EffectivePolicy = %q, want %q", desc.EffectivePolicy, BackendEnvOnly) + } + if desc.DisplayName == "" { + t.Fatal("DisplayName should not be empty") + } + if !desc.Ready { + t.Fatalf("Ready = %v, want true", desc.Ready) + } + if desc.ReadyError != nil { + t.Fatalf("ReadyError = %v, want nil", desc.ReadyError) + } +} + +func TestDescribeRuntimeReportsUnavailableBitwardenAsNotReady(t *testing.T) { + withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) { + switch { + case len(args) == 1 && args[0] == "--version": + return []byte("2026.1.0\n"), nil + case len(args) == 1 && args[0] == "status": + return []byte(`{"status":"locked"}`), nil + default: + return nil, errors.New("unexpected bitwarden invocation") + } + }) + + desc, err := DescribeRuntime(DescribeRuntimeOptions{ + ServiceName: "graylog-mcp", + Shell: "fish", + ExecutableResolver: func() (string, error) { + return filepath.Join(string(filepath.Separator), "opt", "graylog-mcp", "bin", "graylog-mcp"), nil + }, + ManifestLoader: func(startDir string) (manifest.File, string, error) { + return manifest.File{ + SecretStore: manifest.SecretStore{BackendPolicy: string(BackendBitwardenCLI)}, + }, filepath.Join(startDir, manifest.DefaultFile), nil + }, + }) + if err != nil { + t.Fatalf("DescribeRuntime returned error: %v", err) + } + + if desc.DeclaredPolicy != BackendBitwardenCLI { + t.Fatalf("DeclaredPolicy = %q, want %q", desc.DeclaredPolicy, BackendBitwardenCLI) + } + if desc.EffectivePolicy != BackendBitwardenCLI { + t.Fatalf("EffectivePolicy = %q, want %q", desc.EffectivePolicy, BackendBitwardenCLI) + } + if desc.Ready { + t.Fatalf("Ready = %v, want false", desc.Ready) + } + if !errors.Is(desc.ReadyError, ErrBWLocked) { + t.Fatalf("ReadyError = %v, want ErrBWLocked", desc.ReadyError) + } + if !strings.Contains(desc.ReadyError.Error(), "set -x BW_SESSION (bw unlock --raw)") { + t.Fatalf("ReadyError = %v, want fish remediation", desc.ReadyError) + } +} + +func TestPreflightFromManifestReturnsTypedStatusAndRemediation(t *testing.T) { + withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) { + switch { + case len(args) == 1 && args[0] == "--version": + return []byte("2026.1.0\n"), nil + case len(args) == 1 && args[0] == "status": + return []byte(`{"status":"locked"}`), nil + default: + return nil, errors.New("unexpected bitwarden invocation") + } + }) + + report, err := PreflightFromManifest(PreflightOptions{ + ServiceName: "graylog-mcp", + Shell: "fish", + ExecutableResolver: func() (string, error) { + return filepath.Join(string(filepath.Separator), "opt", "graylog-mcp", "bin", "graylog-mcp"), nil + }, + ManifestLoader: func(startDir string) (manifest.File, string, error) { + return manifest.File{ + SecretStore: manifest.SecretStore{BackendPolicy: string(BackendBitwardenCLI)}, + }, filepath.Join(startDir, manifest.DefaultFile), nil + }, + }) + if err != nil { + t.Fatalf("PreflightFromManifest returned error: %v", err) + } + + if report.Status != PreflightStatusFail { + t.Fatalf("Status = %q, want %q", report.Status, PreflightStatusFail) + } + if !strings.Contains(strings.ToLower(report.Summary), "locked") { + t.Fatalf("Summary = %q, want lock hint", report.Summary) + } + if !strings.Contains(report.Remediation, "set -x BW_SESSION (bw unlock --raw)") { + t.Fatalf("Remediation = %q, want fish remediation", report.Remediation) + } +} + +func TestFormatBackendStatusIncludesDeclaredEffectiveAndReadiness(t *testing.T) { + line := FormatBackendStatus(RuntimeDescription{ + ManifestSource: "/opt/graylog-mcp/mcp.toml", + DeclaredPolicy: BackendBitwardenCLI, + EffectivePolicy: BackendBitwardenCLI, + DisplayName: "Bitwarden CLI", + Ready: false, + ReadyError: ErrBWLocked, + }) + + for _, needle := range []string{ + "declared=bitwarden-cli", + "effective=bitwarden-cli", + "display=Bitwarden CLI", + "ready=false", + "source=/opt/graylog-mcp/mcp.toml", + "error=", + } { + if !strings.Contains(line, needle) { + t.Fatalf("line = %q, want substring %q", line, needle) + } + } +} diff --git a/secretstore/store.go b/secretstore/store.go index 2b9bb5f..7b1b787 100644 --- a/secretstore/store.go +++ b/secretstore/store.go @@ -37,6 +37,7 @@ type Options struct { KWalletAppID string KWalletFolder string BitwardenCommand string + Shell string } type Store interface {