Skip to content

Conversation

DvdGiessen
Copy link

@DvdGiessen DvdGiessen commented Jul 30, 2025

This pull request is a (multiple allowed):

  • bug fix
  • change of existing behavior
  • new feature

Checklist

  • User stories are present (given, when, then format)
  • Unit tests are passing
  • Selenium tests are passing
  • Check style is not triggering new error or warning

The bug

If a system doesn't have GnuPG v2 installed using with the name gpg on the PATH, Passbolt doesn't work.

(In my case, my Unix system already had GnuPG v1 installed under the gpg name, and the correct binary name that should be used was gpg2.)

There is a setting for the path to the GPG binary: passbolt.gpg.program. This setting was not being passed to the PhpGnupg() constructor, thus it did not do anything.

What I did

The PhpGnupg() constructor (documentation) supports passing an array of options for setting the paths it should use. I added this array, populated with all options that were configured in the configuration.

Note the keyring path was already passed via a environment variable, so it did work before, but explicitly passing it seemed like a clearer solution than skipping it.

How I tested it

I didn't see any relevant tests to update, and was not quite sure how to best test this in a unit test. Potentially we could set the path to something non-existent and observe that it doesn't work anymore, but that doesn't seem like a very useful test. This fix might be straightforward enough to not need a specific test.

I did test it in my a self-hosted version which needed this fix to run. With the correct path inserted here everything seems to work.

Note: The health check in GpgHealthCheck.php is also broken because it uses a hardcoded command instead of using the passbolt.gpg.program setting, thus claiming it cannot find gpg. I was less sure how to correctly fix this. Prepending the setting to a hardcoded command is possible, but looks a bit risky to me for command injection (even if the source is a relatively trusted config file).

Uncommited change
diff --git a/src/Service/Healthcheck/Environment/GpgHealthcheck.php b/src/Service/Healthcheck/Environment/GpgHealthcheck.php
index 4b31dc7254..5e06fdb34d 100644
--- a/src/Service/Healthcheck/Environment/GpgHealthcheck.php
+++ b/src/Service/Healthcheck/Environment/GpgHealthcheck.php
@@ -21,11 +21,12 @@ use App\Service\Healthcheck\HealthcheckCliInterface;
 use App\Service\Healthcheck\HealthcheckServiceCollector;
 use App\Service\Healthcheck\HealthcheckServiceInterface;
 use App\Utility\CommandRunner;
+use Cake\Core\Configure;
 
 class GpgHealthcheck implements HealthcheckServiceInterface, HealthcheckCliInterface
 {
-    private const COMMAND_GPG = 'gpg --version | grep gpg';
-    private const COMMAND_LIBGCRYPT = 'gpg --version | grep libgcrypt';
+    private const COMMAND_GPG = ' --version | grep gpg';
+    private const COMMAND_LIBGCRYPT = ' --version | grep libgcrypt';
 
     /**
      * Status of this health check if it is passed or failed.
@@ -43,8 +44,9 @@ class GpgHealthcheck implements HealthcheckServiceInterface, HealthcheckCliInter
      */
     public function check(): HealthcheckServiceInterface
     {
-        $this->gpgVersion = $this->runCommand(self::COMMAND_GPG);
-        $this->libgcryptVersion = $this->runCommand(self::COMMAND_LIBGCRYPT);
+        $gpgProgram = Configure::read('passbolt.gpg.program');
+        $this->gpgVersion = $this->runCommand($gpgProgram . self::COMMAND_GPG);
+        $this->libgcryptVersion = $this->runCommand($gpgProgram . self::COMMAND_LIBGCRYPT);
         $this->status = $this->gpgVersion !== null && $this->libgcryptVersion !== null;
 
         return $this;
@@ -95,7 +97,8 @@ class GpgHealthcheck implements HealthcheckServiceInterface, HealthcheckCliInter
      */
     public function getHelpMessage(): array|string|null
     {
-        return [__('See `{0}` and `{1}`.', self::COMMAND_GPG, self::COMMAND_LIBGCRYPT)];
+        $gpgProgram = Configure::read('passbolt.gpg.program');
+        return [__('See `{0}` and `{1}`.', $gpgProgram . self::COMMAND_GPG, $gpgProgram . self::COMMAND_LIBGCRYPT)];
     }
 
     /**

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

1 similar comment
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

2 participants