diff --git a/secretstore/bitwarden.go b/secretstore/bitwarden.go index 3de1960..9c580b8 100644 --- a/secretstore/bitwarden.go +++ b/secretstore/bitwarden.go @@ -50,6 +50,8 @@ type bitwardenStore struct { command string serviceName string debug bool + lookupEnv func(string) (string, bool) + shell string cache *bitwardenCache } @@ -86,6 +88,8 @@ func newBitwardenStore(options Options, policy BackendPolicy, serviceName string command: command, serviceName: serviceName, debug: debugEnabled, + lookupEnv: options.LookupEnv, + shell: options.Shell, cache: newBitwardenCache(bitwardenCacheOptions{ ServiceName: serviceName, Session: session, @@ -95,39 +99,41 @@ func newBitwardenStore(options Options, policy BackendPolicy, serviceName string }), } - if _, err := store.execute("verify bitwarden CLI availability", nil, "--version"); err != nil { + return store, nil +} + +func verifyBitwardenCLIReady(options Options) error { + command := strings.TrimSpace(options.BitwardenCommand) + if command == "" { + command = defaultBitwardenCommand + } + debugEnabled := isBitwardenDebugEnabled(options.BitwardenDebug) + + if _, err := runBitwardenCommand(command, debugEnabled, nil, "--version"); err != nil { if errors.Is(err, exec.ErrNotFound) { - return nil, fmt.Errorf( - "secret backend policy %q requires bitwarden CLI command %q in PATH: %w", - policy, + return fmt.Errorf( + "requires bitwarden CLI command %q in PATH: %w", command, - ErrBackendUnavailable, + errors.Join(ErrBackendUnavailable, err), ) } - return nil, fmt.Errorf( - "secret backend policy %q cannot verify bitwarden CLI command %q: %w", - policy, + return fmt.Errorf( + "cannot verify bitwarden CLI command %q: %w", command, errors.Join(ErrBackendUnavailable, err), ) } - if err := EnsureBitwardenReady(Options{ - BitwardenCommand: command, - BitwardenDebug: debugEnabled, - 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", - policy, + if err := EnsureBitwardenReady(options); err != nil { + return fmt.Errorf( + "cannot use bitwarden CLI command %q right now: %w", command, errors.Join(ErrBackendUnavailable, err), ) } - return store, nil + return nil } func EnsureBitwardenReady(options Options) error { @@ -252,6 +258,10 @@ func detectShellFlavor(shellHint string) string { func (s *bitwardenStore) SetSecret(name, label, secret string) error { secretName := s.scopedName(name) + if err := s.ensureReady(); err != nil { + return fmt.Errorf("prepare bitwarden CLI for saving secret %q: %w", name, err) + } + item, payload, err := s.findItem(secretName, name) switch { case errors.Is(err, ErrNotFound): @@ -316,6 +326,10 @@ func (s *bitwardenStore) GetSecret(name string) (string, error) { } } + if err := s.ensureReady(); err != nil { + return "", fmt.Errorf("prepare bitwarden CLI for reading secret %q: %w", name, err) + } + _, payload, err := s.findItem(secretName, name) if err != nil { return "", err @@ -334,6 +348,10 @@ func (s *bitwardenStore) GetSecret(name string) (string, error) { func (s *bitwardenStore) DeleteSecret(name string) error { secretName := s.scopedName(name) + if err := s.ensureReady(); err != nil { + return fmt.Errorf("prepare bitwarden CLI for deleting secret %q: %w", name, err) + } + item, _, err := s.findItem(secretName, name) if errors.Is(err, ErrNotFound) { if s.cache != nil { @@ -365,6 +383,15 @@ func (s *bitwardenStore) scopedName(name string) string { return fmt.Sprintf("%s/%s", s.serviceName, name) } +func (s *bitwardenStore) ensureReady() error { + return verifyBitwardenCLIReady(Options{ + BitwardenCommand: s.command, + BitwardenDebug: s.debug, + LookupEnv: s.lookupEnv, + Shell: s.shell, + }) +} + type bitwardenResolvedItem struct { item bitwardenListItem payload map[string]any diff --git a/secretstore/bitwarden_cache.go b/secretstore/bitwarden_cache.go index a2a9e55..7990e8b 100644 --- a/secretstore/bitwarden_cache.go +++ b/secretstore/bitwarden_cache.go @@ -32,6 +32,8 @@ const ( bitwardenCacheContextScope = "mcp-framework bitwarden cache backend scope v1" ) +var bitwardenUserCacheDir = os.UserCacheDir + type bitwardenCacheOptions struct { ServiceName string Session string @@ -355,7 +357,7 @@ func (c *bitwardenCache) cacheContext(secretName, scopedName string) string { } func resolveBitwardenCacheDir(serviceName string) string { - cacheRoot, err := os.UserCacheDir() + cacheRoot, err := bitwardenUserCacheDir() if err != nil { return "" } diff --git a/secretstore/bitwarden_cache_test.go b/secretstore/bitwarden_cache_test.go index 5901229..d26097d 100644 --- a/secretstore/bitwarden_cache_test.go +++ b/secretstore/bitwarden_cache_test.go @@ -129,3 +129,13 @@ func TestBitwardenCacheExpiresEntries(t *testing.T) { t.Fatalf("expired cache hit = %q, want miss", got) } } + +func withBitwardenUserCacheDir(t *testing.T, resolver func() (string, error)) { + t.Helper() + + previous := bitwardenUserCacheDir + bitwardenUserCacheDir = resolver + t.Cleanup(func() { + bitwardenUserCacheDir = previous + }) +} diff --git a/secretstore/bitwarden_test.go b/secretstore/bitwarden_test.go index 1860bc5..cafdfc3 100644 --- a/secretstore/bitwarden_test.go +++ b/secretstore/bitwarden_test.go @@ -34,23 +34,29 @@ func TestOpenSupportsBitwardenCLIBackendPolicy(t *testing.T) { if _, ok := store.(*bitwardenStore); !ok { t.Fatalf("store type = %T, want *bitwardenStore", store) } - if !fakeCLI.versionChecked { - t.Fatal("expected bitwarden CLI version check") + if fakeCLI.versionChecked { + t.Fatal("Open should not check bitwarden CLI version before a cache miss or write") } - if !fakeCLI.statusChecked { - t.Fatal("expected bitwarden CLI status check") + if fakeCLI.statusChecked { + t.Fatal("Open should not check bitwarden CLI status before a cache miss or write") } } -func TestOpenBitwardenCLIReturnsUnavailableWhenCommandIsMissing(t *testing.T) { +func TestBitwardenStoreMissReturnsUnavailableWhenCommandIsMissing(t *testing.T) { + withBitwardenSession(t) withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) { return nil, &exec.Error{Name: command, Err: exec.ErrNotFound} }) - _, err := Open(Options{ + store, err := Open(Options{ ServiceName: "email-mcp", BackendPolicy: BackendBitwardenCLI, }) + if err != nil { + t.Fatalf("Open returned error: %v", err) + } + + _, err = store.GetSecret("api-token") if err == nil { t.Fatal("expected error") } @@ -59,26 +65,28 @@ func TestOpenBitwardenCLIReturnsUnavailableWhenCommandIsMissing(t *testing.T) { } } -func TestOpenBitwardenCLIFailsWhenSessionIsMissing(t *testing.T) { +func TestBitwardenStoreMissFailsWhenSessionIsMissing(t *testing.T) { fakeCLI := newFakeBitwardenCLI("bw") withBitwardenRunner(t, fakeCLI.run) - _, err := Open(Options{ + store, err := Open(Options{ ServiceName: "email-mcp", BackendPolicy: BackendBitwardenCLI, LookupEnv: func(name string) (string, bool) { return "", false }, }) + if err != nil { + t.Fatalf("Open returned error: %v", err) + } + + _, err = store.GetSecret("api-token") if err == nil { t.Fatal("expected error") } if !errors.Is(err, ErrBWLocked) { t.Fatalf("error = %v, want ErrBWLocked", err) } - if !errors.Is(err, ErrBackendUnavailable) { - t.Fatalf("error = %v, want ErrBackendUnavailable", err) - } } func TestEnsureBitwardenReadyGuidesLoginAndUnlock(t *testing.T) { @@ -404,6 +412,81 @@ func TestBitwardenStoreGetSecretUsesMemoryCache(t *testing.T) { } } +func TestBitwardenStoreDiskCacheHitSkipsBitwardenCLI(t *testing.T) { + withBitwardenSession(t) + cacheRoot := t.TempDir() + withBitwardenUserCacheDir(t, func() (string, error) { + return cacheRoot, nil + }) + + cache := newBitwardenCache(bitwardenCacheOptions{ + ServiceName: "email-mcp", + Session: os.Getenv("BW_SESSION"), + TTL: defaultBitwardenCacheTTL, + CacheDir: resolveBitwardenCacheDir("email-mcp"), + Enabled: true, + }) + cache.store("api-token", "email-mcp/api-token", "secret-from-cache") + + withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) { + return nil, fmt.Errorf("unexpected bitwarden invocation: %v", args) + }) + + store, err := Open(Options{ + ServiceName: "email-mcp", + BackendPolicy: BackendBitwardenCLI, + }) + 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 != "secret-from-cache" { + t.Fatalf("GetSecret = %q, want secret-from-cache", value) + } +} + +func TestBitwardenStoreCacheMissChecksReadinessLazily(t *testing.T) { + withBitwardenSession(t) + withBitwardenUserCacheDir(t, func() (string, error) { + return t.TempDir(), nil + }) + fakeCLI := newFakeBitwardenCLI("bw") + fakeCLI.itemsByID["item-1"] = fakeBitwardenItem{ + ID: "item-1", + Name: "email-mcp/api-token", + Secret: "secret-v1", + MarkerService: "email-mcp", + MarkerSecretName: "api-token", + } + withBitwardenRunner(t, fakeCLI.run) + + store, err := Open(Options{ + ServiceName: "email-mcp", + BackendPolicy: BackendBitwardenCLI, + }) + if err != nil { + t.Fatalf("Open returned error: %v", err) + } + if fakeCLI.statusChecked { + t.Fatal("Open should not check bitwarden status") + } + + value, err := store.GetSecret("api-token") + if err != nil { + t.Fatalf("GetSecret returned error: %v", err) + } + if value != "secret-v1" { + t.Fatalf("GetSecret = %q, want secret-v1", value) + } + if !fakeCLI.statusChecked { + t.Fatal("cache miss should check bitwarden status before CLI lookup") + } +} + func TestBitwardenStoreCacheDisabledByEnv(t *testing.T) { withBitwardenSession(t) t.Setenv("MCP_FRAMEWORK_BITWARDEN_CACHE", "0") @@ -522,20 +605,33 @@ func TestExecuteBitwardenCLIClassifiesLoginError(t *testing.T) { func TestOpenBitwardenCLIDebugFromEnvPrintsBitwardenCalls(t *testing.T) { withBitwardenSession(t) + withBitwardenUserCacheDir(t, func() (string, error) { + return t.TempDir(), nil + }) fakeCLI := newFakeBitwardenCLI("bw") + fakeCLI.itemsByID["item-1"] = fakeBitwardenItem{ + ID: "item-1", + Name: "email-mcp/api-token", + Secret: "secret-v1", + MarkerService: "email-mcp", + MarkerSecretName: "api-token", + } withBitwardenRunner(t, fakeCLI.run) t.Setenv("MCP_FRAMEWORK_BITWARDEN_DEBUG", "1") var logs bytes.Buffer withBitwardenDebugOutput(t, &logs) - _, err := Open(Options{ + store, err := Open(Options{ ServiceName: "email-mcp", BackendPolicy: BackendBitwardenCLI, }) if err != nil { t.Fatalf("Open returned error: %v", err) } + if _, err := store.GetSecret("api-token"); err != nil { + t.Fatalf("GetSecret returned error: %v", err) + } text := logs.String() if !strings.Contains(text, "bw --version") { @@ -669,6 +765,9 @@ func withBitwardenSession(t *testing.T) { t.Helper() sessionName := strings.NewReplacer("/", "-", " ", "-").Replace(t.Name()) t.Setenv("BW_SESSION", "test-session-"+sessionName) + withBitwardenUserCacheDir(t, func() (string, error) { + return t.TempDir(), nil + }) } func withBitwardenRunner( diff --git a/secretstore/runtime.go b/secretstore/runtime.go index f0894a9..7604b99 100644 --- a/secretstore/runtime.go +++ b/secretstore/runtime.go @@ -91,6 +91,17 @@ func DescribeRuntime(options DescribeRuntimeOptions) (RuntimeDescription, error) desc.EffectivePolicy = effective desc.DisplayName = BackendDisplayName(effective) } + if desc.EffectivePolicy == BackendBitwardenCLI { + if err := verifyBitwardenCLIReady(Options{ + BitwardenCommand: options.BitwardenCommand, + BitwardenDebug: options.BitwardenDebug, + LookupEnv: options.LookupEnv, + Shell: options.Shell, + }); err != nil { + desc.Ready = false + desc.ReadyError = err + } + } return desc, nil }