Install endpoints that leverage Package Manager + core APIs

exec("composer require drupal/$module_id --with-all-dependencies", $install_results, $status);

This is risky to do in production. The Automatic Updates initiative is working on how to do this safely, and includes a package_manager module for that within the 2.x branch of https://www.drupal.org/project/automatic_updates. That code is still beta-level, not stable.

If this MR gets merged with the simple exec() call, and not by using AU's Package Manager, then I think the form that enables this setting should include warnings to only enable this for demos/testing, and not on a real site. I'd suggest also adding a hook_requirements() that checks if this setting is enabled and if so, also adds that warning to Drupal's status report.

It’s doing some validation here https://git.drupalcode.org/project/project_browser/-/merge_requests/154/... to address security issues, and marking things as experimental.

It’s adding options to disable and enable as needed so some of the feedback is somehow addressed.

I think this POC is to really show the capabilities of the module.

(edit as I misunderstood this part)It doesn’t need to be a requirement at all. It could just show the options if they are available AND enabled, but no need to require anything to install the module.

Small change required to check for the new enable/disable flag to avoid running things if we query the endpoint directly and `ui_install` is false. Right now, even if the setting is false, it would run the commands.

@fjgarlin, @effulgentsia is a member of the Drupal Security Team (edit: as am I). As such, his recommendation for security issues, even for an experimental module. The above update is very dangerous, and should not be committed until it has been refactored to depend on the toolchain Automatic Updates will provide.

Failure to follow the Security Team's recommendations could lead to a project losing security coverage or being marked unsupported. Let's especially not risk that for this very important strategic initiative, please. :)

Per #10 and #13, I am marking this issue postponed on Autoupdates' solution. The MVP for Project Browser should not include UI downloads.

@xjm, not a problem. I understand, I was mostly explaining how the commands were sanitized, but I also understand the significance of adding an "exec" command into the code and the recommendation made by @effulgentsia. It also makes sense to coordinate with Automatic Updates, since that's the problem that they're also trying to solve.

Apologies if my comment was taken the wrong way, I was only explaining the code, but not addressing in any way the significance of the comment made.

Looking forward to the day when these two initiatives formally "meet" :-)

Edit: and thank you both for chipping in and the great work that you do. We’re adding many features and sometimes get too eager to get more shipped

Looking forward to the day when these two initiatives formally "meet" :-)

Yeah me too! It is going to improve Drupal in a fundamental way. Thanks for your work on this.

I also chatted with @effulgentsia a bit more and wanted to make it clear that if there is value for folks here in experimenting with the UI patterns we want to have for this, and with workflows etc., I think that's great, and MRs and patches still work on postponed issues. It's just that we shouldn't commit said work until we have the backend part solved properly.

It is clear that we cannot continue with the current approach, but we could bring everything to a safer state, or at least make a plan for it.
Based on the above feedback, a list of remaining tasks could be:

☐ Remove "exec" calls as it is dangerous.
* It is used to download modules and we should use package_manager for this.
* It is used to enable modules and we can use ModuleHandler.

☐ Leverage "package_manager" module which comes with "automatic updates". As this is in beta stage, we should:
* Check if the module is available and if it isn't, don't offer the functionality and inform the user (in the admin page?)
* If the module is enabled, we should probably still have our own toggle, defaulting to "disabled", and warn the user if they enable it that the feature is experimental.
* If it's enabled, then we could show a warning message on the PB landing page, and also display a warning on the status report using "hook_requirements" as suggested by @effulgentsia

☐ Address current outstanding feedback:
* The endpoint to download/enable should check for the status of the PB toggle and not do anything if it's disabled
* We should also check that the given module is "safe". PluginInterface does NOT have a method for it yet, it could be a method called "isModuleSafe", where we pass a "machine_name" and the plugin returns a bool.
** An example could be that some modules could have the "revoked" security coverage status, and these are filtered out in the list, but a malicious user could create an endpoint that actually installs one of those modules

If there are more things that you can think of please add them to the list so we have a plan to follow. Maybe update the description with this info?

And finally, validation from @effulgentsia and/or @xjm on the plan, or feedback on whether this is the right approach or not, would be awesome.

I am getting below error while trying to install module with GUI:

Installation failed, deleting ./composer.json.

In RequireCommand.php line 210:
                                                                               
  No composer.json present in the current directory (./composer.json), this m  
  ay be the cause of the following exception.                                  
                                                                               

In PackageDiscoveryTrait.php line 375:
                                                                               
  Could not find a matching version of package drupal/accessible_media_embed.  
   Check the package spelling, your version constraint and that the package i  
  s available in a stability which matches your minimum-stability (stable).    
                                                                               

require [--dev] [--dry-run] [--prefer-source] [--prefer-dist] [--prefer-install PREFER-INSTALL] [--fixed] [--no-suggest] [--no-progress] [--no-update] [--no-install] [--update-no-dev] [-w|--update-with-dependencies] [-W|--update-with-all-dependencies] [--with-dependencies] [--with-all-dependencies] [--ignore-platform-req IGNORE-PLATFORM-REQ] [--ignore-platform-reqs] [--prefer-stable] [--prefer-lowest] [--sort-packages] [-o|--optimize-autoloader] [-a|--classmap-authoritative] [--apcu-autoloader] [--apcu-autoloader-prefix APCU-AUTOLOADER-PREFIX] [--] [<packages>...]

Basically I am getting error while running below command through terminal or GUI
composer require drupal/accessible_media_embed --with-all-dependencies
And composer install is working for composer require 'drupal/accessible_media_embed:^1.0@beta'

Which version of Drupal is that? Is that using the package_manager module? Could it be your local install? I just tried that command with my local install and it works without issues.

➜  composer require drupal/accessible_media_embed --with-all-dependencies

Using version ^1.0@beta for drupal/accessible_media_embed
./composer.json has been updated
Running composer update drupal/accessible_media_embed --with-all-dependencies
Gathering patches for root package.
Loading composer repositories with package information                                                          Info from https://repo.packagist.org: #StandWithUkraine
Updating dependencies
Lock file operations: 3 installs, 0 updates, 0 removals
  - Locking drupal/accessible_media_embed (1.0.0-beta2)
  - Locking drupal/embed (1.5.0)
  - Locking drupal/entity_embed (1.2.0)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 4 installs, 0 updates, 0 removals
  - Downloading drupal/embed (1.5.0)
  - Downloading drupal/entity_embed (1.2.0)
  - Downloading drupal/accessible_media_embed (1.0.0-beta2)
...

I am using Drupal 9.3.16 on local setup without package_manager module.

Ok, in that case, I wouldn't worry too much for now as we know that we need to replace the exec commands for package_manager ones. The backend part of this issue still needs a lot of work (see #17 and #18), so it's a big WIP.

Two things wrong with the command used:
- no stability flag
- wrong option

- No stabilty flag: The module doesn't have stable releases. By default only stable releases are allowed. Unless your project set minimum-stability, which normally is a bad idea! Suggest @beta as stability, disallowing alpha!

- wrong option: On the require command the option is --update-with-[all-]dependencies on the update command it is --with-dependencies. You might want to use -w|-W as those shortcuts are the same. Not convinced it is the right thing to do in this case. Don't like it that it updates something I didn't ask for.

+1 on keeping the branch alive and just not merging it without backend re-work. But keeping it alive to flush out issues on the front-end side is a great idea. We can ask Site Builder sub-committee for feedback on the UI for installation. This might need to be spun up to test by them, I'll see if I can get @shaal to drop a link in this issue to fire up DrupalPod with this branch's HEAD.

I generated a link to test the issue-branch install-via-ui.
Click on the link to test it in DrupalPod -
https://gitpod.io/#DP_PROJECT_NAME=project_browser,DP_ISSUE_FORK=project...

That info could be added in the "BrowserController" as we're already asking for module information in the "getModuleStatuses()" function.

Feature works fine with some glitches. I was able to download colorbox and metatag module using UI. Logs and glitch screen stuck screenshot is attached. Great work @bnjmnm.

We need to ensure we're only dependent on package_manager and not automatic_updates. The only reason it currently is AU is due to test runner fails, so let's ensure we fix that (either locally or upstream, if needed).

Settings to needs work for suggestions in MR

After talking with @bnjmnm I reopened #3245770: Create a service to composer install via package_manager from Automatic Updates to do only the package_manager integration in a single issue. That issue is now more narrowly scoped than this issue and we it is committed this issue could be update to use the Installer service that it provides to do the Composer installs

Needs work for MR comments.

Addressed most of the feedback and added comments to the rest. Putting up for re-review.

@chrisfromredfin asked over in a thread on the drupal slack in the project browser channel if i could comment in here (even though i thought it wouldn't be beneficial - but he said "the testing is still useful for the existing issue because your testing can help prove that applying the initial MR has not broken anything")

I've applied the merge request to an install of Drupal 9.5. successfully with composer patches locally. alongside i've composer required and installed automatic_updates and package_manager after i read the description of the experimental checkbox in /admin/config/development/project_browser . but on the project browser main page the download and install button still lead to either the modal or forward to the extend page.

I haven't updated the merge request after my previous comment. silencing the validator was a bit tricky until i realized that the node modules folder for svelt in project browser was preventing it from running through. i've simply removed the node modules folder for now. after that i went through the steps outlined in the issue summary. that way i was able to successfully install the paragraphs module:

and one additional observation. after succesfully installing the paragraphs module i've requested a stage id for installing paragraphs again /admin/modules/project_browser/install-begin/drupal/paragraphs and i got a new stage id in return {"phase":"create","status":0,"stage_id":"wIYQb81C8zPTaaJO5-gh3gnzDRsteP9NiWCIRX_dIPE"}

One odd thing. I am still on the same drupal 9.5 install (with php 8.1) from yesterday where i was able to install the paragraphs module. I wanted to try to install another module now to test something else. I've accessed admin/modules/project_browser/install-begin/drupal/pathauto . But i got the following message returned: {"message":"The install stage is locked by a process outside of Project Browser","unlock_url":""}

I've then visisted: admin/modules/project_browser/install-readiness (thought after everything was fine yesterday with no changes it would be still ready for installs today). but i run into:

The website encountered an unexpected error. Please try again later.

LogicException: Drupal\package_manager\Stage::getStageDirectory() cannot be called because the stage has not been created or claimed. in Drupal\package_manager\Stage->getStageDirectory() (line 659 of modules/contrib/automatic_updates/package_manager/src/Stage.php).
Drupal\package_manager\Validator\StagedDBUpdateValidator->hasStagedUpdates(Object, Object) (Line: 176)
Drupal\package_manager\Validator\StagedDBUpdateValidator->getExtensionsWithDatabaseUpdates(Object) (Line: 76)
Drupal\package_manager\Validator\StagedDBUpdateValidator->checkForStagedDatabaseUpdates(Object, 'Drupal\package_manager\Event\StatusCheckEvent', Object)
call_user_func(Array, Object, 'Drupal\package_manager\Event\StatusCheckEvent', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object) (Line: 64)
Drupal\project_browser\Controller\InstallReadinessController->checkReadiness()
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 580)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 159)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 707)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

but if i go via the admin interface to admin/modules/automatic-updateno errors due to failed validator tests are shown. it happens with the state of the merge request i've tested with yesterday. i've also updated to the current state (there were two commits since the last time) and retested. same result.

thank you for working on a fix. I've applied the latest state of the merge request and cleared the caches with drush. a few observations:

1. I've accessed admin/modules/project_browser/install-readiness leaving the node_modules folder in the svelte folder of project_browser and the other time removed it. both times the output was the same: {"pm_validation":false,"stage_available":false}

2. when accessing admin/modules/project_browser/install-begin/drupal/pathauto i got {"message":"An install staging area claimed by Project Browser exists but has expired. You may unlock the stage and try the install again.","unlock_url":"\/admin\/modules\/project_browser\/install\/unlock?token=bkHtPn6oQ2iKBHflVw120RwDTxJdoRLQ17l5aDS7uJU"} but i've never got a stage_id in a previous request

3. when i try to unlock the stage again with the provided code admin/modules/project_browser/install/unlock?token=bkHtPn6oQ2iKBHflVw120RwDTxJdoRLQ17l5aDS7uJUi get Access denied - You are not authorized to access this page.

4. i've tried to initialize an install of a module i'Ve never tried to install before with admin/modules/project_browser/install-begin/drupal/danse and i also get: {"message":"An install staging area claimed by Project Browser exists but has expired. You may unlock the stage and try the install again.","unlock_url":"\/admin\/modules\/project_browser\/install\/unlock?token=bkHtPn6oQ2iKBHflVw120RwDTxJdoRLQ17l5aDS7uJU"}

uhhh i guess i've figured out why the stage was not available. as you can see i've removed the backslashes from the response in #68.2 and instead used the string in #68.3 ( i've assumed only the forward slashes are necessary for the path - which turned out wrong in this case - apologies, i shouldnt make assumption and better follow the instructions in the output instead :/ ).

with the stage cleared and available again everything is behaving as expected. also installing several modules in a row worked now. i'll leave the status to needs review cuz i havent done a code review and only tested manually.

Ship it.

Fixing credit

Merged! What a huge accomplishment, after incredible effort and collaboration.