feat: cache bitwarden secret reads
This commit is contained in:
parent
85da274772
commit
fd08615950
3 changed files with 204 additions and 7 deletions
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
|
|
|
|||
|
|
@ -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{
|
||||
|
|
|
|||
Loading…
Reference in a new issue