diff --git a/scaffold/scaffold.go b/scaffold/scaffold.go index 1856f3c..2d54d99 100644 --- a/scaffold/scaffold.go +++ b/scaffold/scaffold.go @@ -1537,6 +1537,7 @@ import ( "os" "path/filepath" "strings" + "sync" "gitea.lclr.dev/AI/mcp-framework/bootstrap" "gitea.lclr.dev/AI/mcp-framework/cli" @@ -1816,14 +1817,16 @@ func (r Runtime) runConfigTest(ctx context.Context, inv bootstrap.Invocation) er stdout = os.Stdout } + secretStoreFactory := memoizeSecretStoreFactory(r.openSecretStore) + report := cli.RunDoctor(ctx, cli.DoctorOptions{ ConfigCheck: cli.NewConfigCheck(r.ConfigStore), - SecretStoreCheck: cli.SecretStoreAvailabilityCheck(r.openSecretStore), + SecretStoreCheck: cli.SecretStoreAvailabilityCheck(secretStoreFactory), SecretBackendPolicy: r.activeBackendPolicy(), RequiredSecrets: []cli.DoctorSecret{ {Name: r.SecretName, Label: "API token"}, }, - SecretStoreFactory: r.openSecretStore, + SecretStoreFactory: secretStoreFactory, ManifestCheck: r.manifestDoctorCheck(), BitwardenOptions: cli.BitwardenDoctorOptions{ LookupEnv: os.LookupEnv, @@ -1841,6 +1844,25 @@ func (r Runtime) runConfigTest(ctx context.Context, inv bootstrap.Invocation) er return nil } +func memoizeSecretStoreFactory(factory func() (secretstore.Store, error)) func() (secretstore.Store, error) { + if factory == nil { + return nil + } + + var ( + once sync.Once + store secretstore.Store + err error + ) + + return func() (secretstore.Store, error) { + once.Do(func() { + store, err = factory() + }) + return store, err + } +} + func (r Runtime) runUpdate(ctx context.Context, inv bootstrap.Invocation) error { stdout := inv.Stdout if stdout == nil { diff --git a/scaffold/scaffold_test.go b/scaffold/scaffold_test.go index 5880090..2e351eb 100644 --- a/scaffold/scaffold_test.go +++ b/scaffold/scaffold_test.go @@ -76,6 +76,8 @@ func TestGenerateCreatesRecommendedSkeleton(t *testing.T) { "ManifestSource", "ManifestCheck: r.manifestDoctorCheck()", "SecretBackendPolicy: r.activeBackendPolicy()", + "secretStoreFactory := memoizeSecretStoreFactory(r.openSecretStore)", + "func memoizeSecretStoreFactory(factory func() (secretstore.Store, error)) func() (secretstore.Store, error) {", "cli.WriteSetupSecretVerified", } { if !strings.Contains(string(appGo), snippet) { diff --git a/secretstore/bitwarden.go b/secretstore/bitwarden.go index 4d0b6f6..439b146 100644 --- a/secretstore/bitwarden.go +++ b/secretstore/bitwarden.go @@ -211,7 +211,7 @@ func detectShellFlavor(shellHint string) string { func (s *bitwardenStore) SetSecret(name, label, secret string) error { secretName := s.scopedName(name) - item, err := s.findItem(secretName, name) + item, payload, err := s.findItem(secretName, name) switch { case errors.Is(err, ErrNotFound): template, err := s.itemTemplate() @@ -239,11 +239,6 @@ func (s *bitwardenStore) SetSecret(name, label, secret string) error { return err } - payload, err := s.itemByID(item.ID) - if err != nil { - return err - } - setBitwardenSecretPayload(payload, s.serviceName, name, secretName, label, secret) encoded, err := s.encodePayload(payload) if err != nil { @@ -266,12 +261,7 @@ func (s *bitwardenStore) SetSecret(name, label, secret string) error { func (s *bitwardenStore) GetSecret(name string) (string, error) { secretName := s.scopedName(name) - item, err := s.findItem(secretName, name) - if err != nil { - return "", err - } - - payload, err := s.itemByID(item.ID) + _, payload, err := s.findItem(secretName, name) if err != nil { return "", err } @@ -286,7 +276,7 @@ func (s *bitwardenStore) GetSecret(name string) (string, error) { func (s *bitwardenStore) DeleteSecret(name string) error { secretName := s.scopedName(name) - item, err := s.findItem(secretName, name) + item, _, err := s.findItem(secretName, name) if errors.Is(err, ErrNotFound) { return nil } @@ -311,7 +301,12 @@ func (s *bitwardenStore) scopedName(name string) string { return fmt.Sprintf("%s/%s", s.serviceName, name) } -func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenListItem, error) { +type bitwardenResolvedItem struct { + item bitwardenListItem + payload map[string]any +} + +func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenListItem, map[string]any, error) { output, err := s.execute( fmt.Sprintf("list bitwarden items for secret %q", secretName), nil, @@ -321,15 +316,15 @@ func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenLi secretName, ) if err != nil { - return bitwardenListItem{}, err + return bitwardenListItem{}, nil, err } if strings.TrimSpace(string(output)) == "" { - return bitwardenListItem{}, ErrNotFound + return bitwardenListItem{}, nil, ErrNotFound } var items []bitwardenListItem if err := json.Unmarshal(output, &items); err != nil { - return bitwardenListItem{}, fmt.Errorf("decode bitwarden items for secret %q: %w", secretName, err) + return bitwardenListItem{}, nil, fmt.Errorf("decode bitwarden items for secret %q: %w", secretName, err) } matches := make([]bitwardenListItem, 0, len(items)) @@ -344,42 +339,47 @@ func (s *bitwardenStore) findItem(secretName, rawSecretName string) (bitwardenLi } if len(matches) == 0 { - return bitwardenListItem{}, ErrNotFound + return bitwardenListItem{}, nil, ErrNotFound } - markedMatches := make([]bitwardenListItem, 0, len(matches)) - legacyMatches := make([]bitwardenListItem, 0, len(matches)) + markedMatches := make([]bitwardenResolvedItem, 0, len(matches)) + legacyMatches := make([]bitwardenResolvedItem, 0, len(matches)) for _, item := range matches { payload, err := s.itemByID(item.ID) if err != nil { - return bitwardenListItem{}, err + return bitwardenListItem{}, nil, err + } + + resolved := bitwardenResolvedItem{ + item: item, + payload: payload, } if bitwardenItemMatchesMarkers(payload, s.serviceName, rawSecretName) { - markedMatches = append(markedMatches, item) + markedMatches = append(markedMatches, resolved) continue } - legacyMatches = append(legacyMatches, item) + legacyMatches = append(legacyMatches, resolved) } switch len(markedMatches) { case 0: switch len(legacyMatches) { case 0: - return bitwardenListItem{}, ErrNotFound + return bitwardenListItem{}, nil, ErrNotFound case 1: - return legacyMatches[0], nil + return legacyMatches[0].item, legacyMatches[0].payload, nil default: - return bitwardenListItem{}, fmt.Errorf( + return bitwardenListItem{}, nil, fmt.Errorf( "multiple legacy bitwarden items match secret %q for service %q", secretName, s.serviceName, ) } case 1: - return markedMatches[0], nil + return markedMatches[0].item, markedMatches[0].payload, nil default: - return bitwardenListItem{}, fmt.Errorf( + return bitwardenListItem{}, nil, fmt.Errorf( "multiple bitwarden items share marker for secret %q and service %q", secretName, s.serviceName, diff --git a/secretstore/bitwarden_test.go b/secretstore/bitwarden_test.go index b71e832..5da8fe2 100644 --- a/secretstore/bitwarden_test.go +++ b/secretstore/bitwarden_test.go @@ -337,6 +337,38 @@ func TestBitwardenStoreFallsBackToSingleLegacyItemWithoutMarkers(t *testing.T) { } } +func TestBitwardenStoreGetSecretReadsSelectedItemOnlyOnce(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) + } + + 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.getItemCalls != 1 { + t.Fatalf("bw get item count = %d, want 1", fakeCLI.getItemCalls) + } +} + func TestBitwardenFindItemTreatsEmptyListOutputAsNotFound(t *testing.T) { store := &bitwardenStore{command: "bw", serviceName: "email-mcp"} withBitwardenRunner(t, func(command string, stdin []byte, args ...string) ([]byte, error) { @@ -346,7 +378,7 @@ func TestBitwardenFindItemTreatsEmptyListOutputAsNotFound(t *testing.T) { return nil, fmt.Errorf("unexpected args: %v", args) }) - _, err := store.findItem("email-mcp/api-token", "api-token") + _, _, err := store.findItem("email-mcp/api-token", "api-token") if !errors.Is(err, ErrNotFound) { t.Fatalf("error = %v, want ErrNotFound", err) } @@ -527,6 +559,7 @@ type fakeBitwardenCLI struct { status string versionChecked bool statusChecked bool + getItemCalls int } type fakeBitwardenItem struct { @@ -568,6 +601,7 @@ func (f *fakeBitwardenCLI) run(command string, stdin []byte, args ...string) ([] case len(args) == 4 && args[0] == "list" && args[1] == "items" && args[2] == "--search": return f.handleListItems(args[3]) case len(args) == 3 && args[0] == "get" && args[1] == "item": + f.getItemCalls++ return f.handleGetItem(args[2]) case len(args) == 3 && args[0] == "get" && args[1] == "template" && args[2] == "item": return []byte(`{"type":2,"name":"","notes":"","secureNote":{"type":0},"fields":[]}`), nil