diff --git a/secretstore/bitwarden.go b/secretstore/bitwarden.go index 362c019..3de1960 100644 --- a/secretstore/bitwarden.go +++ b/secretstore/bitwarden.go @@ -50,6 +50,7 @@ type bitwardenStore struct { command string serviceName string debug bool + cache *bitwardenCache } type bitwardenListItem struct { @@ -68,12 +69,6 @@ func newBitwardenStore(options Options, policy BackendPolicy, serviceName string } debugEnabled := isBitwardenDebugEnabled(options.BitwardenDebug) - store := &bitwardenStore{ - command: command, - serviceName: serviceName, - debug: debugEnabled, - } - if _, err := EnsureBitwardenSessionEnv(BitwardenSessionOptions{ ServiceName: serviceName, }); err != nil { @@ -85,6 +80,21 @@ func newBitwardenStore(options Options, policy BackendPolicy, serviceName string ) } + session, _ := os.LookupEnv(bitwardenSessionEnvName) + cacheEnabled := !options.DisableBitwardenCache && !bitwardenCacheDisabledByEnv() + store := &bitwardenStore{ + command: command, + serviceName: serviceName, + debug: debugEnabled, + cache: newBitwardenCache(bitwardenCacheOptions{ + ServiceName: serviceName, + Session: session, + TTL: defaultBitwardenCacheTTL, + CacheDir: resolveBitwardenCacheDir(serviceName), + Enabled: cacheEnabled, + }), + } + if _, err := store.execute("verify bitwarden CLI availability", nil, "--version"); err != nil { if errors.Is(err, exec.ErrNotFound) { return nil, fmt.Errorf( @@ -265,6 +275,10 @@ func (s *bitwardenStore) SetSecret(name, label, secret string) error { ); err != nil { return err } + if s.cache != nil { + s.cache.invalidate(name, secretName) + s.cache.store(name, secretName, secret) + } return nil case err != nil: return err @@ -287,11 +301,21 @@ func (s *bitwardenStore) SetSecret(name, label, secret string) error { return err } + if s.cache != nil { + s.cache.invalidate(name, secretName) + s.cache.store(name, secretName, secret) + } return nil } func (s *bitwardenStore) GetSecret(name string) (string, error) { secretName := s.scopedName(name) + if s.cache != nil { + if secret, ok := s.cache.load(name, secretName); ok { + return secret, nil + } + } + _, payload, err := s.findItem(secretName, name) if err != nil { return "", err @@ -302,6 +326,9 @@ func (s *bitwardenStore) GetSecret(name string) (string, error) { return "", fmt.Errorf("bitwarden item for secret %q does not contain field %q", name, bitwardenSecretFieldName) } + if s.cache != nil { + s.cache.store(name, secretName, secret) + } return secret, nil } @@ -309,6 +336,9 @@ func (s *bitwardenStore) DeleteSecret(name string) error { secretName := s.scopedName(name) item, _, err := s.findItem(secretName, name) if errors.Is(err, ErrNotFound) { + if s.cache != nil { + s.cache.invalidate(name, secretName) + } return nil } if err != nil { @@ -325,6 +355,9 @@ func (s *bitwardenStore) DeleteSecret(name string) error { return err } + if s.cache != nil { + s.cache.invalidate(name, secretName) + } return nil } diff --git a/secretstore/bitwarden_test.go b/secretstore/bitwarden_test.go index 4b64c57..1860bc5 100644 --- a/secretstore/bitwarden_test.go +++ b/secretstore/bitwarden_test.go @@ -370,6 +370,124 @@ func TestBitwardenStoreGetSecretReadsSelectedItemOnlyOnce(t *testing.T) { } } +func TestBitwardenStoreGetSecretUsesMemoryCache(t *testing.T) { + withBitwardenSession(t) + 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) + } + + for i := 0; i < 2; i++ { + value, err := store.GetSecret("api-token") + if err != nil { + t.Fatalf("GetSecret #%d returned error: %v", i+1, err) + } + if value != "secret-v1" { + t.Fatalf("GetSecret #%d = %q, want secret-v1", i+1, value) + } + } + if fakeCLI.getItemCalls != 1 { + t.Fatalf("bw get item count = %d, want 1 with memory cache", fakeCLI.getItemCalls) + } +} + +func TestBitwardenStoreCacheDisabledByEnv(t *testing.T) { + withBitwardenSession(t) + t.Setenv("MCP_FRAMEWORK_BITWARDEN_CACHE", "0") + 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) + } + + for i := 0; i < 2; i++ { + if _, err := store.GetSecret("api-token"); err != nil { + t.Fatalf("GetSecret #%d returned error: %v", i+1, err) + } + } + if fakeCLI.getItemCalls != 2 { + t.Fatalf("bw get item count = %d, want 2 when env disables cache", fakeCLI.getItemCalls) + } +} + +func TestBitwardenStoreSetSecretRefreshesCache(t *testing.T) { + withBitwardenSession(t) + fakeCLI := newFakeBitwardenCLI("bw") + withBitwardenRunner(t, fakeCLI.run) + + store, err := Open(Options{ + ServiceName: "email-mcp", + BackendPolicy: BackendBitwardenCLI, + }) + if err != nil { + t.Fatalf("Open returned error: %v", err) + } + if err := store.SetSecret("api-token", "API token", "secret-v1"); err != nil { + t.Fatalf("SetSecret v1 returned error: %v", err) + } + if got, err := store.GetSecret("api-token"); err != nil || got != "secret-v1" { + t.Fatalf("GetSecret after v1 = %q, %v; want secret-v1, nil", got, err) + } + if err := store.SetSecret("api-token", "API token", "secret-v2"); err != nil { + t.Fatalf("SetSecret v2 returned error: %v", err) + } + if got, err := store.GetSecret("api-token"); err != nil || got != "secret-v2" { + t.Fatalf("GetSecret after v2 = %q, %v; want secret-v2, nil", got, err) + } +} + +func TestBitwardenStoreDeleteSecretInvalidatesCache(t *testing.T) { + withBitwardenSession(t) + fakeCLI := newFakeBitwardenCLI("bw") + withBitwardenRunner(t, fakeCLI.run) + + store, err := Open(Options{ + ServiceName: "email-mcp", + BackendPolicy: BackendBitwardenCLI, + }) + if err != nil { + t.Fatalf("Open returned error: %v", err) + } + if err := store.SetSecret("api-token", "API token", "secret-v1"); err != nil { + t.Fatalf("SetSecret returned error: %v", err) + } + if _, err := store.GetSecret("api-token"); err != nil { + t.Fatalf("GetSecret before delete returned error: %v", err) + } + if err := store.DeleteSecret("api-token"); err != nil { + t.Fatalf("DeleteSecret returned error: %v", err) + } + _, err = store.GetSecret("api-token") + if !errors.Is(err, ErrNotFound) { + t.Fatalf("GetSecret after delete error = %v, want ErrNotFound", err) + } +} + func TestBitwardenFindItemTreatsEmptyListOutputAsNotFound(t *testing.T) { store := &bitwardenStore{command: "bw", serviceName: "email-mcp"} withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) { @@ -549,7 +667,8 @@ func TestOpenFromManifestSupportsBitwardenCLIBackendPolicy(t *testing.T) { func withBitwardenSession(t *testing.T) { t.Helper() - t.Setenv("BW_SESSION", "test-session") + sessionName := strings.NewReplacer("/", "-", " ", "-").Replace(t.Name()) + t.Setenv("BW_SESSION", "test-session-"+sessionName) } func withBitwardenRunner( diff --git a/secretstore/manifest_open_test.go b/secretstore/manifest_open_test.go index 0a92d37..c60e8c6 100644 --- a/secretstore/manifest_open_test.go +++ b/secretstore/manifest_open_test.go @@ -138,6 +138,51 @@ func TestResolveManifestPolicyPreservesBitwardenCacheDisable(t *testing.T) { } } +func TestOpenFromManifestAppliesBitwardenCacheDisable(t *testing.T) { + withBitwardenSession(t) + 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) + + cacheDisabled := false + store, err := OpenFromManifest(OpenFromManifestOptions{ + ServiceName: "email-mcp", + ExecutableResolver: func() (string, error) { + return filepath.Join(string(filepath.Separator), "opt", "email-mcp", "bin", "email-mcp"), nil + }, + ManifestLoader: func(startDir string) (manifest.File, string, error) { + return manifest.File{ + SecretStore: manifest.SecretStore{ + BackendPolicy: string(BackendBitwardenCLI), + BitwardenCache: &cacheDisabled, + }, + }, filepath.Join(startDir, manifest.DefaultFile), nil + }, + }) + if err != nil { + t.Fatalf("OpenFromManifest returned error: %v", err) + } + + for i := 0; i < 2; i++ { + value, err := store.GetSecret("api-token") + if err != nil { + t.Fatalf("GetSecret #%d returned error: %v", i+1, err) + } + if value != "secret-v1" { + t.Fatalf("GetSecret #%d = %q, want secret-v1", i+1, value) + } + } + if fakeCLI.getItemCalls != 2 { + t.Fatalf("bw get item count = %d, want 2 when manifest disables cache", fakeCLI.getItemCalls) + } +} + func TestOpenFromManifestReturnsExecutableResolutionError(t *testing.T) { execErr := errors.New("boom") _, err := OpenFromManifest(OpenFromManifestOptions{