Skip to content

refactor(keyretriever): reduce duplicate master-key WARN logs#589

Merged
moonD4rk merged 2 commits intomainfrom
refactor/keyretriever-log-noise
Apr 25, 2026
Merged

refactor(keyretriever): reduce duplicate master-key WARN logs#589
moonD4rk merged 2 commits intomainfrom
refactor/keyretriever-log-noise

Conversation

@moonD4rk
Copy link
Copy Markdown
Owner

Summary

  • Demote in-chain retriever failures from WARN to Debug
  • GcoredumpRetriever falls through silently on internal failures
  • Dedupe master-key WARN per browser instead of per profile
  • Clarify "exit status 128 ()" with empty stderr message

Test plan

  • golangci-lint run clean
  • go test ./... passes
  • Manual smoke on macOS multi-profile: 46 WARN → 3 WARN

Closes #588

- Demote in-chain retriever failures to Debug
- GcoredumpRetriever logs at Debug and falls through silently
- Dedupe master-key WARN per browser instead of per profile
- Clarify "exit status 128 ()" with empty stderr message

Closes #588
Copilot AI review requested due to automatic review settings April 25, 2026 13:47
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 25, 2026

Codecov Report

❌ Patch coverage is 16.66667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.60%. Comparing base (50c4ea8) to head (9386ca6).

Files with missing lines Patch % Lines
browser/chromium/chromium.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #589      +/-   ##
==========================================
+ Coverage   71.25%   73.60%   +2.35%     
==========================================
  Files          61       61              
  Lines        3294     2815     -479     
==========================================
- Hits         2347     2072     -275     
+ Misses        757      553     -204     
  Partials      190      190              
Flag Coverage Δ
unittests 73.60% <16.66%> (+2.35%) ⬆️

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.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors Chromium master-key retrieval logging to reduce repeated WARN-level messages during multi-profile extraction, particularly on macOS when key retrieval commonly fails in expected ways (e.g., no keychain access / not root).

Changes:

  • Demotes per-retriever failures inside ChainRetriever from WARN to Debug to avoid noisy “expected failure” logs.
  • Updates macOS GcoredumpRetriever to silently fall through (return (nil, nil)) on internal failures while logging only at Debug.
  • Deduplicates “master key retrieval” WARN logs across Chromium profiles by emitting a WARN once and subsequent failures as Debug; also improves security command error clarity when stderr is empty.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
crypto/keyretriever/keyretriever_darwin.go Adjusts macOS retrievers to avoid repeated warning-worthy failures; improves security error messaging.
crypto/keyretriever/keyretriever.go Demotes in-chain retriever failure logs from WARN to Debug.
browser/chromium/chromium.go Adds per-browser WARN deduplication for master-key retrieval failures.

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

Comment thread browser/chromium/chromium.go Outdated
… per browser name

Address Copilot review: glob-expanded configs (e.g., MSIX/UWP browsers
with publisher hash suffixes) yield multiple installations sharing a
BrowserName but with different UserDataDir. Include UserDataDir in the
dedupe key so each installation still emits its first WARN.
@moonD4rk moonD4rk merged commit 15680c1 into main Apr 25, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce duplicate WARN logs from master-key retrieval chain

3 participants