fix: kill stale GPG agents before key generation#892
fix: kill stale GPG agents before key generation#892nogeenharrie wants to merge 1 commit intomainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
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 207-213: The two Executor::executeBlocking calls are invoking gpg
incorrectly and calling gpgconf as an argument; fix by deriving the gpgconf
executable path from gpgPath (use QFileInfo to get gpgPath's directory) then
call Executor::executeBlocking(gpgPath, {"--kill", "gpg-agent"}) should be
removed or replaced with the proper gpgconf invocation: build gpgconfPath from
QFileInfo(gpgPath).absolutePath() + QDir::separator() + "gpgconf" and call
Executor::executeBlocking(gpgconfPath, {"--kill", "gpg-agent"}); add `#include`
<QFileInfo> to the includes.
🪄 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: 882a7afc-c438-4aab-be45-ca4c6ad895fa
📒 Files selected for processing (1)
src/pass.cpp
| // Kill any stale GPG agents that might be holding locks on the key database | ||
| // This helps avoid "database locked" timeouts during key generation | ||
| QString gpgPath = QtPassSettings::getGpgExecutable(); | ||
| if (!gpgPath.isEmpty()) { | ||
| Executor::executeBlocking(gpgPath, {"--kill", "gpg-agent"}); | ||
| Executor::executeBlocking(gpgPath, {"gpgconf", "--kill", "gpg-agent"}); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the correct command to kill gpg-agent using gpgconf?
💡 Result:
The correct command to kill gpg-agent using gpgconf is gpgconf --kill gpg-agent.
Citations:
- 1: https://www.gnupg.org/documentation/manuals/gnupg/Invoking-GPG_002dAGENT.html
- 2: https://www.gnupg.org/(en)/documentation/manuals/gnupg24/gpgconf.1.html
- 3: https://www.gnupg.org/(en)/documentation/manuals/gnupg24/gpg-agent.1.html
- 4: https://www.gnupg.org/documentation/manuals/gnupg26/gpgconf.1.html
🏁 Script executed:
# Check for existing includes in src/pass.cpp around the code area
head -50 src/pass.cpp | grep -E "^#include"Repository: IJHack/QtPass
Length of output: 259
🏁 Script executed:
# Search for gpgconf references in the codebase
rg "gpgconf" --type cpp --type hRepository: IJHack/QtPass
Length of output: 145
🏁 Script executed:
# Search for QFileInfo usage in the codebase
rg "QFileInfo" --type cpp --type h -A 2 | head -30Repository: IJHack/QtPass
Length of output: 1429
🏁 Script executed:
# Search for QtPassSettings methods to see if there's a pattern for executable paths
rg "class QtPassSettings|getGpg|Executable" --type cpp --type h -A 1 | head -50Repository: IJHack/QtPass
Length of output: 2967
Both agent-kill commands are incorrect and will fail silently.
Line 211 executes gpg --kill gpg-agent, which is invalid—GPG does not support the --kill option. Line 212 executes gpg gpgconf --kill gpg-agent (with "gpgconf" as an argument to the gpg binary) instead of running the gpgconf executable directly. The correct command is gpgconf --kill gpg-agent.
Derive the gpgconf path from gpgPath by extracting its directory. This pattern is already used elsewhere in the codebase (e.g., tst_util.cpp). Add #include <QFileInfo> to the includes and update the code:
Fix: correctly invoke gpgconf to kill the agent
// Kill any stale GPG agents that might be holding locks on the key database
// This helps avoid "database locked" timeouts during key generation
QString gpgPath = QtPassSettings::getGpgExecutable();
if (!gpgPath.isEmpty()) {
- Executor::executeBlocking(gpgPath, {"--kill", "gpg-agent"});
- Executor::executeBlocking(gpgPath, {"gpgconf", "--kill", "gpg-agent"});
+ // Derive gpgconf path from gpg path (they are typically in the same directory)
+ QFileInfo gpgInfo(gpgPath);
+ QString gpgconfPath = gpgInfo.absolutePath() + QDir::separator() + "gpgconf";
+#ifdef Q_OS_WIN
+ gpgconfPath += ".exe";
+#endif
+ if (QFile::exists(gpgconfPath)) {
+ Executor::executeBlocking(gpgconfPath, {"--kill", "gpg-agent"});
+ }
}Add to includes section:
`#include` <QFileInfo>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/pass.cpp` around lines 207 - 213, The two Executor::executeBlocking calls
are invoking gpg incorrectly and calling gpgconf as an argument; fix by deriving
the gpgconf executable path from gpgPath (use QFileInfo to get gpgPath's
directory) then call Executor::executeBlocking(gpgPath, {"--kill", "gpg-agent"})
should be removed or replaced with the proper gpgconf invocation: build
gpgconfPath from QFileInfo(gpgPath).absolutePath() + QDir::separator() +
"gpgconf" and call Executor::executeBlocking(gpgconfPath, {"--kill",
"gpg-agent"}); add `#include` <QFileInfo> to the includes.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Before generating GPG keys, kill any stale gpg-agent processes that might be holding locks on the key database. This helps prevent 'timeout while waiting on locked key database' errors. The issue occurs when another GPG process (possibly from another application) holds a lock on the key database, causing key generation to fail with timeout errors.
86cc302 to
b9d8446
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/pass.cpp (1)
209-212:⚠️ Potential issue | 🔴 CriticalInvoke
gpgconfdirectly; current command line is malformed.Line 211 currently runs
gpgwith"gpgconf"as an argument, which is not the same as executinggpgconf --kill gpg-agent. This means stale-agent cleanup may not happen and the lock-timeout failure can persist.Proposed fix
QString gpgPath = QtPassSettings::getGpgExecutable(); if (!gpgPath.isEmpty()) { - Executor::executeBlocking(gpgPath, {"gpgconf", "--kill", "gpg-agent"}); + QFileInfo gpgInfo(gpgPath); + QString gpgconfPath = gpgInfo.absolutePath() + QDir::separator() + "gpgconf"; +#ifdef Q_OS_WIN + gpgconfPath += ".exe"; +#endif + const int rc = Executor::executeBlocking(gpgconfPath, {"--kill", "gpg-agent"}); +#ifdef QT_DEBUG + if (rc != 0) { + dbg() << "Failed to kill gpg-agent:" << gpgconfPath << rc; + } +#endif }What is the correct GnuPG command to terminate gpg-agent, and must it be executed via the gpgconf executable (not as an argument to gpg)?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/pass.cpp` around lines 209 - 212, The code is invoking the gpg binary with "gpgconf" as an argument (gpgPath + {"gpgconf","--kill","gpg-agent"}) which is wrong; instead execute the gpgconf program directly. Replace the call that uses gpgPath and args {"gpgconf","--kill","gpg-agent"} so it invokes the gpgconf executable (e.g. use a QtPassSettings::getGpgConfExecutable() if available or the literal "gpgconf") and call Executor::executeBlocking(gpgconfPath, {"--kill","gpg-agent"}) so gpg-agent is terminated properly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/pass.cpp`:
- Around line 209-212: The code is invoking the gpg binary with "gpgconf" as an
argument (gpgPath + {"gpgconf","--kill","gpg-agent"}) which is wrong; instead
execute the gpgconf program directly. Replace the call that uses gpgPath and
args {"gpgconf","--kill","gpg-agent"} so it invokes the gpgconf executable (e.g.
use a QtPassSettings::getGpgConfExecutable() if available or the literal
"gpgconf") and call Executor::executeBlocking(gpgconfPath,
{"--kill","gpg-agent"}) so gpg-agent is terminated properly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1cab2960-c85d-4025-bdf8-85c9af7c919a
📒 Files selected for processing (1)
src/pass.cpp
Summary
Fixes issue #815 - GPG key generation fails due to a timeout while waiting on a locked key database.
Problem
When another GPG process (from another application) holds a lock on the key database, key generation fails with:
Solution
Before generating GPG keys, kill any stale gpg-agent processes that might be holding locks:
This releases any stale locks on the GPG key database before attempting key generation.
Testing
Summary by CodeRabbit