Problem/Motivation

In Drupal core 11.2, Package Manager has added support for a "direct-write" mode, which allows changes to be made directly on the live site: #3503699: Allow Package Manager to operate directly on the live code base

Individual implementers of Package Manager need to specifically opt into it for it if they want to support it. Should Project Browser do it? The usefulness is clear, but this issue is for us to fully consider the risks and do what we might do to mitigate them.

What are some of the risks? As @catch pointed out in #3503699-38: Allow Package Manager to operate directly on the live code base (paraphrased for clarity):

Pathauto 2.x removes ctools as a composer dependency . This means that when sites eventually update to pathauto 2.x, they'll need to either composer require drupal/ctools or uninstall it (unless they happen to have another module installed that depends on it).

...if you install a project that depends on pathauto 2.x, and you have pathauto 1.x installed, then composer might update you to pathauto 2.x even though the command in composer require which could also result in ctools removal.

There might well be others.

Issue fork project_browser-3525507

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

phenaproxima created an issue. See original summary.

tolstoydotcom’s picture

tolstoydotcom
Location L.A.
commented

Letting a public web server write to code directories is a huge security risk. It's a hacker's dream. On local environments it's not an issue.

The idea that everyone is going to have a git workflow with a dev site and a developer running ddev on a local machine is a fantasy. If the idea is to appeal to people who aren't going to use the command line, then the only safe alternative is something like https://www.drupal.org/project/sheephole_helper Since it's open source, what's the objection to using that instead of encouraging an unsafe server environment?

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

Letting a public web server write to code directories is a huge security risk. It's a hacker's dream. On local environments it's not an issue.

That's why direct-write mode needs to be enabled per environment (in a setting), and it explicitly warns about being risky and not recommended on production.

Project Browser should opt into it specifically to support local development.

tolstoydotcom’s picture

tolstoydotcom
Location L.A.
commented

Those who've worked with smaller clients will realize what will happen: they'll take the risk. Then, some will get hacked. Then, Drupal will get a bad reputation about being insecure.

There are still lots of sites running D7/8/9, including major sites like large universities and government sites. There are countless thousands of sites running vulnerable versions of WordPress. They're taking a risk or they don't know any better.

Drupal should make it as easy as possible to be as secure as possible. That's a strong selling point. Drupal should not make it easy for people to create insecure environments.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

To clarify something: just because Project Browser opts into supporting direct-write does NOT mean it is automatically enabled!

Direct-write has a two-part opt-in process. From the change record:

To operate directly on the live site, two conditions must be met.

  1. A new setting, package_manager_allow_direct_write, must be set to TRUE.
  2. Individual subclasses of \Drupal\package_manager\SandboxManagerBase must have the \Drupal\package_manager\Attribute\AllowDirectWrite attribute applied to them.

Supporting is not the same as allowing.

By supporting direct-write, Project Browser will not be assuming that all environments are local. The site owner needs to actually allow direct-write to be used at all by altering settings.php.

The best practice here would be to enable it in settings.local.php only, which should not normally be deployed to a production environment. Therefore, done correctly, local environments will be able to use direct-write (which is what it was intended for), and production environments will not.

Of course, you as a site owner could enable it on a production environment too, which is something we explicitly warn against. If you do that anyway, against our warnings, you're on your own. Drupal takes this secure-by-default-but-you-can-break-it-if-you-try approach in many places.

phenaproxima opened merge request !807

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Needs review
chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented

In my opinion, yes, in this case the potential benefits outweigh the risks. I support adding this functionality. The real risk with PB direct-operating on the codebase is actually during activation (turning on a bad module that whitescreens your site). This is simply eliminating a copying step and improving performance, especially for things like local environments. For example, I personally would probably use PB to add and enable modules rather than Composer directly, and I'm a seasoned Drupaler. BUT, I don't right now because it's slower.

The only risk I do see with operating on a live site is if our Composer operation results in upgrading a module in-place. But, we have other ways to potentially handle that (an interstitial "here's what operations will happen," modal for example).

+1 on this feature. I think it's needed. FWIW, I (and Leslie) do NOT support surfacing this setting in the UI.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

FWIW, I (and Leslie) do NOT support surfacing this setting in the UI.

Direct-write is not a choice that can be surfaced in the UI. That would imply it is a configuration flag, which it is not. It has to be enabled by a developer -- someone with access to the code base, who can alter settings.php (or, better yet, settings.local.php) physically. That's all by design.

an interstitial "here's what operations will happen," modal for example

We don't have this capability yet except with sandboxing, but if core implements and adopts #3525563: Create a Composer plugin that records what packages are being updated, installed, or removed, then we might be able to get there.

chrisfromredfin’s picture

chrisfromredfin
he/him/his
Primary language English
Location Portland, Maine
commented
Status: Needs review » Fixed

woohoo! huge time-saver. ship it

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.