fix: show user-friendly error when GPG key is expired or invalid#1035
Conversation
When ImitatePass::Insert fails because the recipient key is expired, revoked, or missing, GPG exits non-zero and the raw stderr was shown verbatim in the output panel — not helpful to users who don't read GPG output daily. Add --status-fd 2 to the GPG invocation so machine-readable [GNUPG:] status tokens (KEY_EXPIRED, KEY_REVOKED, NO_PUBKEY, INV_RECP, FAILURE) are included in stderr. These are locale-independent and checked first; case-insensitive substring fallbacks handle GPG builds or configurations that don't emit status tokens. In Pass::finished, when PASS_INSERT fails and a known pattern is found, a translated friendly message is prepended and the [GNUPG:] machine lines are stripped before emitting processErrorExit, so the output pane shows e.g. "Encryption failed: GPG key has expired. Please renew or replace it." followed by the human-readable GPG lines only. The transaction mechanism in ImitatePass::finished already cancels the queued GIT_ADD and GIT_COMMIT commands when GPG fails, so the misleading git pathspec error is not shown. 11 new unit tests cover all status-token paths, all fallback paths, the empty-result case, and priority ordering (token beats fallback). Closes #247 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
CodeAnt AI is reviewing your PR. |
Kody Review CompleteGreat news! 🎉 Keep up the excellent work! 🚀 Kody Guide: Usage and ConfigurationInteracting with Kody
Current Kody ConfigurationReview OptionsThe following review options are enabled or disabled:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughRedirects GPG status output to stderr for insert/encrypt operations and adds Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as "QtPass UI"
participant ImitatePass as "ImitatePass"
participant GPG as "GPG"
participant Pass as "Pass"
participant Display as "Error Display"
User->>UI: request insert/encrypt
UI->>ImitatePass: start insert (GPG args include --status-fd 2)
ImitatePass->>GPG: run encryption process
GPG-->>ImitatePass: exit non-zero + stderr ([GNUPG:] lines + text)
ImitatePass->>Pass: notify finished (PASS_INSERT) with stderr
Pass->>Pass: remove [GNUPG:] lines from stderr
Pass->>Pass: gpgErrorMessage(stderr)
alt token matched
Pass->>Display: emit token-based friendly message
else fallback matched
Pass->>Display: emit substring-based friendly message
else no match
Pass->>Display: emit original stderr via processErrorExit
end
Display->>User: show error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
|
CodeAnt AI finished reviewing your PR. |
|
coverage: 22.061% (+0.3%) from 21.778% — fix/247-gpg-key-expired-error-message into main |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pass.cpp`:
- Around line 450-467: gpgErrorMessage currently checks incorrect GnuPG tokens
and handles INV_RECP too broadly; update the token checks in gpgErrorMessage to
use the actual status-fd tokens KEYEXPIRED and KEYREVOKED (no underscores),
reorder checks so KEYEXPIRED/KEYREVOKED are matched before any INV_RECP branch,
and add specific INV_RECP reason-code checks for "INV_RECP 5" (expired) and
"INV_RECP 4" (revoked) before falling back to a generic INV_RECP message; ensure
all searches use the same QStringLiteral("[GNUPG:] ...") pattern so
expired/revoked cases return the correct localized messages.
In `@tests/auto/util/tst_util.cpp`:
- Around line 157-167: The parser in gpgErrorMessage() currently matches
nonstandard tokens; update its checks in src/pass.cpp to look for "KEYEXPIRED"
and "KEYREVOKED" (instead of "KEY_EXPIRED"/"KEY_REVOKED") and add explicit
handling of "[GNUPG:] INV_RECP" reason codes so that INV_RECP 5 maps to expired
and INV_RECP 4 maps to revoked (leave other INV_RECP codes as generic
invalid-recipient). After changing the parser logic, update the tests in
tests/auto/util/tst_util.cpp (the gpgErrorMessage* fixtures including
gpgErrorMessageKeyExpiredStatusToken, gpgErrorMessageKeyRevokedStatusToken,
gpgErrorMessageInvRecpStatusToken, etc., and the fixtures around lines
1607-1706) to use the real GnuPG status lines ("[GNUPG:] KEYEXPIRED", "[GNUPG:]
KEYREVOKED", and "[GNUPG:] INV_RECP 5" / "[GNUPG:] INV_RECP 4") so the tests
validate the corrected behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 722a2be5-0b56-45df-bf4f-aae6e2156d77
📒 Files selected for processing (4)
src/imitatepass.cppsrc/pass.cppsrc/pass.htests/auto/util/tst_util.cpp
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1035 +/- ##
==========================================
+ Coverage 28.16% 28.53% +0.37%
==========================================
Files 39 39
Lines 3373 3406 +33
==========================================
+ Hits 950 972 +22
- Misses 2423 2434 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Replace KEY_EXPIRED/KEY_REVOKED (which are not real --status-fd tokens) with KEYEXPIRED and KEYREVOKED. Remove REVKEYSIG (signature verification only). Add INV_RECP 5 (expired) and INV_RECP 4 (revoked) checks before the generic INV_RECP fallback. Update tests accordingly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/pass.cpp`:
- Around line 509-525: The code that builds humanErr from err.split('\n') can
leave stray '\r' characters on CRLF output; inside the PASS_INSERT block (the
loop over for (const QString &line : err.split('\n'))), strip or remove any '\r'
from each non-[GNUPG:] line before appending to humanLines (e.g., normalize line
endings or use line = line.trimmed() / line.remove('\r') or split on CRLF-aware
delimiter) so humanErr has no embedded '\r' when passed to emit
processErrorExit(exitCode, ...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5974e3f0-6ec9-4b4a-9710-66c0d0928645
📒 Files selected for processing (2)
src/pass.cpptests/auto/util/tst_util.cpp
Split on '\n' leaves trailing '\r' on CRLF output (Windows/WSL). Remove '\r' from each line before appending to humanLines so the error passed to processErrorExit is clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2 participants
This pull request improves the user experience by providing more descriptive and user-friendly error messages when GPG encryption fails.
Key changes include:
--status-fd 2flag during password insertion.gpgErrorMessagehas been introduced to parse GPG's standard error output. It prioritizes machine-readable[GNUPG:]status tokens to identify specific issues like expired, revoked, or missing GPG keys.This ensures that users receive actionable feedback when GPG keys are expired, revoked, or otherwise invalid, rather than cryptic technical messages.
Summary by CodeRabbit
New Features
Improvements
Tests