- Issue created by @alexpott
- 🇺🇸United States phenaproxima Massachusetts
I think we need this. Right now, the documentation (in Package Manager's hook_help) tells you to use Drush, but this is not always an option for site builders, especially not on cheaper hosting.
Since the executables' locations is already configurable, this would not hurt the status quo.
- Merge request !11904Create a settings form for Package Manager. Mostly AI-generated but it did a good job. → (Open) created by phenaproxima
- First commit to issue fork.
- 🇹🇳Tunisia azizos
I am trying to sign my commit with a verified signature that Drupal GitLab knows. I am already using ssh keys to sign my commits. Probably does not work the same way with Drupal GitLab.
- 🇹🇳Tunisia azizos
@phenaproxima I have a question in my mind. Why do not we have a enabled Package Manager from the extend tab?? I can think about that it is meant for developers only. I want to hear your thoughts.
- 🇺🇸United States phenaproxima Massachusetts
It's hidden because it's not in beta.
- 🇺🇸United States tedbow Ithaca, NY, USA
Does drupal core have any other cases where you can set the path of executable from the UI? Just wondering because I guess you could do ton of damage if you could set this whatever you want
Also the summary doesn't mention rsync
- 🇺🇸United States phenaproxima Massachusetts
Updated the issue summary.
I am not aware of any other place where core allows the paths to executables to be configured, but I can't think of anything else in core which needs to call external executables to work properly.
Maybe we hide it behind a more restrictive permission (
administer software updates
, which has the "restrict access" flag)? - 🇺🇸United States tedbow Ithaca, NY, USA
. I was noting this is something new in #11 not looking to block it. Though we could still add the summary that it is something new and worth the risk to make Package Manager and auto-updates usable on shared hosting
Hostinger currently makes composer 1 and composer 2 available. Composer 1 is composer and Composer 2 is composer2
Did you have to do
which composer2
to actually find the path?So I had to download the phar file and point the project to there to work.
It sounds like if you have to do this already is updating settings.php such a big deal.
Trying to think of admin who has all the skills get composer at a path and/or figure out the current path, where updating settings.php is such a big deal, especially since we get tell you abouting updating settings.php in the status check message
I am not against this just trying figure out the profile of the person we are helping with this
- 🇺🇸United States phenaproxima Massachusetts
I run a multisite that has
composer
andcomposer2
available as two separate executables, for the record. I think that forcing people to alter settings.php -- which can break a lot more things with less recoverability (depending on how you break settings.php, you might not even know what has gone wrong) -- is not very helpful.IS updated.
- 🇬🇧United Kingdom alexpott 🇪🇺🌍
We are helping people on hosts like Hostinger. They have both composer and composer2 installed but neither will work with automatic updates. Once they update their composer2 you would still need to update the path to their composer2 - currently you need to download the phar and place it somewhere yourself. TBH we could have a follow-up with instructions for how to do that because on a lot of cheap hosts it is going to be necessary.
The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- 🇺🇸United States phenaproxima Massachusetts
Looks good - I changed that description to a warning message, which I think is probably the better option (more visible). I removed some of the wording (core UI standards forbid the use of "please" in user-facing text, if I remember correctly), and wrote a test.
The Needs Review Queue Bot → tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.
- Status changed to RTBC
26 days ago 7:18pm 19 May 2025 - 🇺🇸United States tedbow Ithaca, NY, USA
This looks to me. I guess generally if people are ok with the user setting paths to executables then I am ok with it. 'administer software updates' is already very powerful permission and should not be given to many people
- 🇳🇿New Zealand quietone
This should have screens shots so I added some.
This is what came to mind while creating them:
- I think an error is in order if the path does not exist.
- The text 'Configure the paths is at the bottom, where I expected it a the top. Like the 'default indexing settings' at admin/config/search/pages.
- The text 'Package Manager' is the only item with capital case at admin/config. Perhaps a more 'functional' name than the module name should be used. And same for the description.
- The form saves on Enter. Is that what is wanted? Some pages do and some don't and that is annoying to me.
I haven't actually tried out the functionality!
- 🇬🇧United Kingdom catch
. I guess generally if people are ok with the user setting paths to executables then I am ok with it
I think the main risk is via XSS account takeover, and then using that permission to set paths to executables. That's already a problem with project browser though - in that someone could take over an account and install devel on production or similar.
However assuming we do ✨ Require password for certain admin actions Active for this and also for installing new modules, that should be mitigated a lot.
Having said that, does this need additional validation of the string? We could restrict it to only a-z + forward slashes for example.
- 🇬🇧United Kingdom catch
Back to needs work for #25 and possibly the validation in #26.
- 🇺🇸United States phenaproxima Massachusetts
Addressed #25 and #26, except for:
The form saves on Enter. Is that what is wanted? Some pages do and some don't and that is annoying to me.
I very much doubt that this has anything to do with Package Manager itself, it just uses ConfigFormBase like many other settings forms in core do. But maybe this merits a follow-up (in Claro)?
- 🇬🇧United Kingdom mcdruid 🇬🇧🇪🇺
+1 to concerns raised in #26 / #27.
Allowing the path to an executable to be set in the UI is a really valuable way for an attacker to escalate XSS to RCE.
One recent related issue was adding Symfony Mailer transports to core 📌 [PP-1] Add symfony mailer transports to DIC Postponed where I believe it's necessary to set anything non-standard via settings.php.
Validation is really important (noting the value is being checked on submit with
is_executable()
and there are tests showing that command -args type values will fail).I agree that this makes ✨ Require password for certain admin actions Active a high priority security hardening.
- 🇳🇿New Zealand quietone
Added a "Uses" section to the Help.
Also noticed the setting page no longer loads. Is there no test to ensure the functionality of a settings page?
The website encountered an unexpected error. Try again later. InvalidArgumentException: Class "\Drupal\package_manager\Form\SettingsForm" does not exist. in Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition() (line 32 of core/lib/Drupal/Core/DependencyInjection/ClassResolver.php). Drupal\Core\Controller\HtmlFormController->getFormObject() (Line: 58) Drupal\Core\Controller\FormController->getContentResult() call_user_func_array() (Line: 123)
- 🇺🇸United States phenaproxima Massachusetts
There is absolutely a test: https://git.drupalcode.org/project/drupal/-/blob/3dfed6a33787da29769280d...
- 🇩🇪Germany rkoller Nürnberg, Germany
one question, i've checked out the MR and tested in DDEV. Both binaries, composer and rsync, are available within the ddev webcontainer at the default locations, and ssh'ed into the container to make sure that it is the case (*and it is, both are available).
The question i have, when the composer and rsync binaries are available and in default places (
/usr/local/
&/usr/bin/
), is package manager then fully functional with no further actions necessary?At the moment the ui is sending mixed and a bit confusing messages in my testing. For one the path for composer has "auto-detected" appended. strictly speaking a placeholder text is about to inform the user what kind of data could be entered into that field not a status check. it imho can't be taken for granted that a placeholder text informs you about everything is all right, that the binary is available and composer will be available to package manager. and if i would take for granted that auto detect placeholder would inform me about the availability then i would assume that i have to enter a path for rsync because it is not available in the default location in
/usr/bin/
.that placeholder approach leaves a quite a bit of uncertainty to me the user imho.
- 🇬🇧United Kingdom catch
. strictly speaking a placeholder text is about to inform the user what kind of data could be entered into that field not a status check.
Yeah I think that should be moved outside. In general, we don't want people to configure these values if the locations are auto-detected anyway I would expect, because if they can be autodetected differently on local vs remote environments that's better than configuration.
- 🇺🇸United States phenaproxima Massachusetts
Thanks for the feedback - I agree it could be clearer. I changed it so that, if the path is auto-detected, it's made into the placeholder (as a sample value), and a status message tells the user that the path was auto-detected.
- 🇺🇸United States phenaproxima Massachusetts
Changed it a bit more. Now, if auto-detect fails, the field that couldn't be auto-detected is marked as required, so that you as a user don't get the impression that everything is copacetic. Status or warning messages are set for each field, so you know exactly whether or not it worked.
- 🇸🇰Slovakia poker10
I tested the MR manually and found some issues:
1. Initially, package manager auto-detected my composer, but not rsync. The rsync field was therefore required. I entered manually the path to the rsync executable and the field was no longer required after saving the form. Therefore I was able to delete the value again - which is probably not what we want, since it was not auto-detected initially? I think it will be better to keep the required flag always, if the executable was not auto-detected to not allow removing the value.
2. When you save the form with a value in a field which was not auto-detected, after the form is submitted, you get duplicate messages, like:
Status message The path to Composer was automatically detected. The configuration options have been saved. The path to rsync was automatically detected.
Warning message The path to rsync could not be automatically detected.
So the info about rsync path was duplicated. The same issues happened when I removed the rsync path (as mentioned in point 1).
3. When I set the path to rsync manually, I see the message:
The path to rsync was automatically detected.
Which is not correct, as I entered the value manually and it was not detected automatically.
4. On windows - composer path was detected as
C:\ProgramData\ComposerSetup\bin\composer.BAT
. In windows enviroment settings (PATH), there isC:\ProgramData\ComposerSetup\bin
and the composer executable file exists physically with a lowercase extension (not uppercase). Not sure if that is a problem (and if this is caused by SymfonyExecutableFinder). What is a bit worse is, that even when the PHP docs mention, thatis_executable()
function will detect .bat files as executables (see https://www.php.net/manual/en/function.is-executable.php -for BC reasons, files with a .bat or .cmd extension are also considered executable
), when I try to save the form with manually enteredC:\ProgramData\ComposerSetup\bin\composer.BAT
orC:\ProgramData\ComposerSetup\bin\composer.bat
, it fails the IsExecutableConstraintValidator ("C:\ProgramData\ComposerSetup\bin\composer.bat" is not an executable file.
). This is probably not good for windows users.5. Can we consider adding an info somewhere, that removing the value will "reset" the config to the auto-detected value?
Moving this to Needs work for these. Thanks!
- 🇸🇰Slovakia poker10
It would be also great to update the IS, as the images in User interface changes are outdated (for example there is no (auto-detected) in placeholder, the details element does not contain info that you can clear the value to return back to auto-detect and there are additional status/warning messages).
- 🇬🇧United Kingdom catch
Therefore I was able to delete the value again - which is probably not what we want, since it was not auto-detected initially? I think it will be better to keep the required flag always, if the executable was not auto-detected to not allow removing the value.
Wouldn't this make it impossible to remove it from configuration if it becomes detectable on your hosting? Or if you change hosts?
- 🇳🇿New Zealand quietone
I don't know why the form was failing yesterday. I had done a fresh composer install and install of Drupal. Did the same today and it is working for me.
Rebased and changed the description displayed on the config form, using the working of 'File system' as a guide. And update the screenshot in the IS.
I also get this confusing screen when entering a non-existent file.
- 🇸🇰Slovakia poker10
@quietone Thanks! I think we still need to update other screenshots (from the SettingsForm), so keeping this at Needs work for this and also for other issues from #36.
@catch I think that if we base the required state on the check done by SymfonyExecutableFinder/PhpTuf , the field will become not required as soon as the executable is detected again (sorry if I used the wrong word was/is). My idea was, that if the specific executable is not auto-detected, then the required flag should be set until it is auto-detected. In case the specific excecutable is auto-detected, it can be kept as is, without the required flag, so that you can set custom path, or revert to the auto-detected value. I just think that resetting the executable path back to NULL (default value) should not be allowed if the auto-detected path is not available at the moment, because it will reset you to the non-working state regarding that specific executable.
- 🇩🇪Germany rkoller Nürnberg, Germany
We haven't had the time for a full review in today's meeting ( 📌 Drupal Usability Meeting 2025-05-23 Active with @benjifisher, @rkoller, and @simohell), we just got to it during the last ten minutes. We will revisit the issue hopefully next week. But I am writing this comment cuz we stumbled across one noteworthy detail:
while demonstrating the MR, i wanted to demonstrate the detail about when the binary is not available (for that i've simply renamed rsync to rsyncer and composer to composerer within the webcontainer in ddev). when i got to the settings page from the reports page i think ive adjusted the names to the new naming in the executable path fields and saved - then the contradictory message showed together (see the screenshot). problem is when i was trying to reproduce the behavior the settings page behaved odd. the reports page was properly noting when composer was available and when not (when i'Ve renamed it). but on the settings page sometimes the binary was shown as autodetected even though it was renamed, at one time it seeemed even the path and file name got auto adjusted to the renamed file name. have to test some more tomorrow.
- 🇩🇪Germany rkoller Nürnberg, Germany
Usability review
We discussed this issue at 📌 Drupal Usability Meeting 2025-05-30 Active . The recording of the meeting: https://youtu.be/18q8p7xz7VE. For the record, the attendees at the usability meeting were @benjifisher, @rkoller, and @simohell.
We revisited this issue at 📌 Drupal Usability Meeting 2025-06-06 Needs work . That issue will have a link to a recording of the meeting. For the record, the attendees at the second meeting were @rkoller, @simohell, and @worldlinemine.
We have taken a look at the new settings page on
admin/config/system/package-manager
, the change to the check for composer on the status report pageadmin/reports/status
, and simulated a few scenarios by renaming composer and rsync binaries in the DDEV web container, and experimented with overriding those executable path settings via the settings.php file. A few observations:- Having a status message that is reporting that the paths for
composer
andrsync
were automatically detected each and every time a user is visitingadmin/config/system/package-manager
raises many potential questions and might be confusing to the user. One might ask themselves is it necessary to access this settings page first, before package manager is able to autodetect composer and rsync, until then the two binaries remain unavailable to the site and package manager since the auto-detection was not trigged yet? Furthermore one would expect as soon as the two binaries were autodetected the status message wouldn’t show up ever again as long as the path and/or file name don’t change - why is something auto-detected and announced again and again after it was already detected and the executable path set for that location? Having a status message informing the user about a state change, a newly arisen situation, and/or a successfully completed task is the right purpose, but having a redundant message about a task that was already successfully accomplished feels counter productive and leaves a certain uncertainty about the persistence of that autodetected setting. - It seems the accuracy between the auto-detection on
admin/config/system/package-manager
and the composer test onadmin/reports/status
is divergent. Out of the box with/usr/local/bin/composer
available in the DDEV web containeradmin/config/system/package-manager
reports that composer is automatically detected, andadmin/reports/status
reports it checked successfully for composer returning2.8.9 (/usr/local/bin/composer)
. If you rename/usr/local/bin/composer
to/usr/local/bin/composerer
, reloadingadmin/config/system/package-manager
returns again that the path to composer was automatically detected, whileadmin/reports/status
is throwing an error on reload that composer is not found. If you now change the executable path onadmin/config/system/package-manager
to/usr/local/bin/composerer
and save, the status message again returns that composer was automatically detected, same for reloadingadmin/reports/status
, it reports that it successfully checked for composer returning2.8.9 (/usr/local/bin/composer)
. The only time you are able to trigger an error onadmin/config/system/package-manager
is by changing the executable path to/usr/local/bin/composer
while path and binary are still renamed to/usr/local/bin/composerer
. In consequence, the auto detection onadmin/config/system/package-manager
feels not trustworthy compared to the status check onadmin/reports/status
. - If you change the composer executable path from
/usr/local/bin/composer
to another binary that is available in the same directory within the DDEV web container, like/usr/local/bin/node
, the form is still getting validated successfully. - If you set
$config['package_manager.settings']['executables']['rsync'] = '/usr/bin/rsync’;
in thesettings.php
and change the rsync executable path to/usr/bin/rsyncer
onadmin/config/system/package-manager
the following two warnings show up twice:
Warning: Undefined array key "#parents" in Drupal\Core\Form\FormState->getError() (line 1176 of /var/www/html/repos/drupal/core/lib/Drupal/Core/Form/FormState.php). Drupal\Core\Form\FormState->getError() (Line: 166) Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 126) Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 126) Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 26) Drupal\Core\Form\FormErrorHandler->handleFormErrors() (Line: 200) Drupal\Core\Form\FormValidator->finalizeValidation() (Line: 119) Drupal\Core\Form\FormValidator->validateForm() (Line: 585) Drupal\Core\Form\FormBuilder->processForm() (Line: 321) Drupal\Core\Form\FormBuilder->buildForm() (Line: 73) Drupal\Core\Controller\FormController->getContentResult() call_user_func_array() (Line: 123) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 622) Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 183) Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76) Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53) Drupal\Core\StackMiddleware\Session->handle() (Line: 48) Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28) Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32) Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116) Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90) Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48) Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51) Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 53) Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51) Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 715) Drupal\Core\DrupalKernel->handle() (Line: 19) Warning: foreach() argument must be of type array|object, null given in Drupal\Core\Form\FormState->getError() (line 1176 of /var/www/html/repos/drupal/core/lib/Drupal/Core/Form/FormState.php). Drupal\Core\Form\FormState->getError() (Line: 166) Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 126) Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 126) Drupal\Core\Form\FormErrorHandler->setElementErrorsFromFormState() (Line: 26) Drupal\Core\Form\FormErrorHandler->handleFormErrors() (Line: 200) Drupal\Core\Form\FormValidator->finalizeValidation() (Line: 119) Drupal\Core\Form\FormValidator->validateForm() (Line: 585) Drupal\Core\Form\FormBuilder->processForm() (Line: 321) Drupal\Core\Form\FormBuilder->buildForm() (Line: 73) Drupal\Core\Controller\FormController->getContentResult() call_user_func_array() (Line: 123) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 622) Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 121) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97) Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 183) Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76) Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53) Drupal\Core\StackMiddleware\Session->handle() (Line: 48) Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28) Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32) Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 116) Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 90) Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48) Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51) Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 53) Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 51) Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 715) Drupal\Core\DrupalKernel->handle() (Line: 19)
. The warning is also shown if you set the executable path and the settings override to
/usr/bin/rsync
but set a none existing executable path for composer like/usr/local/bin/composerer
. So the warning seems only to show if either of the two executable paths is failing the form validation and one of the two fields is overridden in the settings.php.
The points we came up with:
- We had a clear consensus in both meetings to recommend replacing the status message with a text box (solves point 1). Instead of auto-detecting and creating “noise” every time you visit
admin/config/system/package-manager
, simply display the path the two binaries are available at and auto-detected in a text box. Only return an error message, with the same accuracy as experienced on the reports page (solves point 2), if package manager is unable to auto-detect the binary/binaries. That way the user is always in control and gets the reassurance everything is in place and package manager is fully operational. At the same time it is clearly communicated that it is possible to change/override the executable path(s) - for that the two fields should not use any placeholder text and be empty instead. - In addition to adding the text box, we would also suggest to change the title of the field set underneath from
Executable path
toCustom executable path
to further clarify that you are actually overriding the auto-detected defaults here and/or provide working executable paths in case one or both binaries are not available on the host. - We also had a clear consensus adding the check available for composer on
admin/reports/status
for rsync as well. We’ve already created a follow up issue for that: ✨ Add a runtime requirements test for rsync analogues to composer Active - The warning about the undefined error key in case settings are overridden via the settings.php should be fixed. (solves point 4)
- In the end we wonder if one of the following scenarios we’ve observer and/or discussed have any security implications:
- Adding a different binary, like for example node, for composer and/or rsync (see point 3)
- Adding an executable path to a binary in the sites public or private folder, a location where users are able to upload files.
If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.
- Having a status message that is reporting that the paths for