fix: cache keychain retriever across browser profiles on macOS

[moonD4rk]

Summary

• Share a single KeyRetriever instance across all profiles of the same browser, created once in NewBrowsers() instead of per-profile in getMasterKey()
• Add sync.Once caching to GcoredumpRetriever and SecurityCmdRetriever so each only executes once per browser
• This also makes KeychainPasswordRetriever's existing sync.Once actually effective (previously each profile created a new instance, so the sync.Once never deduplicated)

Closes #544

Test plan

• go build ./cmd/hack-browser-data/
• go test ./...
• golangci-lint run
• Manual test on macOS with multiple Chrome profiles — verify only one password prompt appears

[codecov-commenter]

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.95%. Comparing base (92053b8) to head (af28aab).

Files with missing lines	Patch %	Lines
browser/chromium/chromium.go	66.66%	1 Missing ⚠️

Additional details and impacted files

@@                     Coverage Diff                      @@
##           feat/v2-hbd-architecture     #545      +/-   ##
============================================================
+ Coverage                     67.86%   67.95%   +0.08%     
============================================================
  Files                            48       48              
  Lines                          1581     1582       +1     
============================================================
+ Hits                           1073     1075       +2     
+ Misses                          400      399       -1     
  Partials                        108      108              

Flag	Coverage Δ
unittests	67.95% <66.66%> (+0.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Copilot]

Pull request overview

This PR reduces repeated macOS Keychain prompts when extracting data from multiple Chromium profiles by reusing a single key-retriever chain per browser and adding one-time execution caching inside macOS retrievers.

Changes:

• Instantiate keyretriever.DefaultRetriever(...) once in chromium.NewBrowsers() and reuse it across all discovered profiles.
• Add sync.Once-based caching to GcoredumpRetriever and SecurityCmdRetriever so expensive/interactive retrieval runs only once per retriever instance.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
crypto/keyretriever/keyretriever_darwin.go	Adds sync.Once caching to macOS retrievers to avoid repeated memory dumps / security CLI prompts.
browser/chromium/chromium.go	Shares a single KeyRetriever across all Chromium profiles created by NewBrowsers().

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

GcoredumpRetriever now caches the derived key with a single sync.Once, but RetrieveKey still takes a storage parameter. If the same retriever instance is ever reused for different storage labels, subsequent calls will incorrectly return the first storage’s key (or error). Consider caching per-storage (e.g., map[storage]result with a mutex) or binding/validating the storage value in the retriever instance.

Suggested change

	// The result is cached via sync.Once to avoid repeated memory dumps
	// when multiple profiles share the same retriever instance.
	type GcoredumpRetriever struct {
	once sync.Once
	key []byte
	err error
	}
	func (r *GcoredumpRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
	r.once.Do(func() {
	r.key, r.err = r.retrieveKeyOnce(storage)
	})
	return r.key, r.err
	// Results are cached per storage label to avoid repeated memory dumps
	// when multiple profiles share the same retriever instance.
	type gcoredumpResult struct {
	key []byte
	err error
	}
	type GcoredumpRetriever struct {
	mu sync.Mutex
	results map[string]gcoredumpResult
	}
	func (r *GcoredumpRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
	r.mu.Lock()
	if result, ok := r.results[storage]; ok {
	r.mu.Unlock()
	return result.key, result.err
	}
	r.mu.Unlock()
	key, err := r.retrieveKeyOnce(storage)
	r.mu.Lock()
	if r.results == nil {
	r.results = make(map[string]gcoredumpResult)
	}
	if result, ok := r.results[storage]; ok {
	r.mu.Unlock()
	return result.key, result.err
	}
	r.results[storage] = gcoredumpResult{
	key: key,
	err: err,
	}
	r.mu.Unlock()
	return key, err

[Copilot]

SecurityCmdRetriever caches key/err with a single sync.Once, but RetrieveKey accepts a storage argument. Reusing the same retriever instance for different storage labels would return the key (or error) for the first storage only. Consider per-storage caching or making storage an immutable field of the retriever and rejecting mismatched calls.

Suggested change

	// This may trigger a password dialog on macOS. The result is cached
	// via sync.Once so that multiple profiles sharing the same retriever
	// instance only prompt the user once.
	type SecurityCmdRetriever struct {
	once sync.Once
	key []byte
	err error
	}
	func (r *SecurityCmdRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
	r.once.Do(func() {
	r.key, r.err = r.retrieveKeyOnce(storage)
	})
	return r.key, r.err
	// This may trigger a password dialog on macOS. Results are cached per
	// storage label so that multiple profiles sharing the same retriever
	// instance only prompt the user once for the same storage.
	type SecurityCmdRetriever struct {
	mu sync.Mutex
	results map[string]securityCmdResult
	}
	type securityCmdResult struct {
	key []byte
	err error
	}
	func (r *SecurityCmdRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
	storage = strings.TrimSpace(storage)
	r.mu.Lock()
	if result, ok := r.results[storage]; ok {
	r.mu.Unlock()
	return result.key, result.err
	}
	r.mu.Unlock()
	key, err := r.retrieveKeyOnce(storage)
	r.mu.Lock()
	defer r.mu.Unlock()
	if r.results == nil {
	r.results = make(map[string]securityCmdResult)
	}
	if result, ok := r.results[storage]; ok {
	return result.key, result.err
	}
	r.results[storage] = securityCmdResult{
	key: key,
	err: err,
	}
	return key, err

[Copilot]

Because sync.Once caches failures too, a transient error (e.g., user cancels the Keychain prompt, temporary security CLI failure, timeout) will be permanently reused for the lifetime of this retriever instance with no retry. If you want deduplication without making transient failures sticky, consider only caching on success (or allow retry when the cached err != nil).

Suggested change

	// This may trigger a password dialog on macOS. The result is cached
	// via sync.Once so that multiple profiles sharing the same retriever
	// instance only prompt the user once.
	type SecurityCmdRetriever struct {
	once sync.Once
	key []byte
	err error
	}
	func (r *SecurityCmdRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
	r.once.Do(func() {
	r.key, r.err = r.retrieveKeyOnce(storage)
	})
	return r.key, r.err
	// This may trigger a password dialog on macOS. A successful result is cached
	// so that multiple profiles sharing the same retriever instance only prompt
	// the user once, while transient failures remain retryable.
	type SecurityCmdRetriever struct {
	mu sync.Mutex
	loaded bool
	key []byte
	}
	func (r *SecurityCmdRetriever) RetrieveKey(storage, _ string) ([]byte, error) {
	r.mu.Lock()
	defer r.mu.Unlock()
	if r.loaded {
	return r.key, nil
	}
	key, err := r.retrieveKeyOnce(storage)
	if err != nil {
	return nil, err
	}
	r.key = key
	r.loaded = true
	return r.key, nil