Skip to content

Commit

Permalink
Patch core to not check file owner in update
Browse files Browse the repository at this point in the history
  • Loading branch information
xendk committed Dec 4, 2024
1 parent 28517da commit f5e7071
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 2 deletions.
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,8 @@
"3035578: Add details for AJAX response errors": "https://www.drupal.org/files/issues/2023-07-24/3035578-26.patch",
"3293926: Error decorating non-existent service when inner service's module not installed": "https://git.drupalcode.org/project/drupal/-/commit/a64662a3cea209c106b888ce9ef03cc1808f9ffe.patch",
"Always assume chmod succeeds": "patches/core-chmod-true.patch",
"Debug Form triggering element detection": "patches/form-triggering-element-detection-paragraphs-ee.patch"
"Debug Form triggering element detection": "patches/form-triggering-element-detection-paragraphs-ee.patch",
"Don't check for file owner in update": "patches/dont-check-fileowner.patch"
},
"drupal/date_range_formatter": {
"3309324: Fails when start and end date are identical": "https://www.drupal.org/files/issues/2023-12-22/date_range_formatter_3309324_1.patch"
Expand Down
2 changes: 1 addition & 1 deletion composer.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 52 additions & 0 deletions patches/dont-check-fileowner.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
diff --git a/modules/update/src/Form/UpdateReady.php b/modules/update/src/Form/UpdateReady.php
index 779ad5b5a1..f02098037b 100644
--- a/modules/update/src/Form/UpdateReady.php
+++ b/modules/update/src/Form/UpdateReady.php
@@ -145,12 +145,9 @@ class UpdateReady extends FormBase {
];
}

- // If the owner of the last directory we extracted is the same as the
- // owner of our configuration directory (e.g. sites/default) where we're
- // trying to install the code, there's no need to prompt for FTP/SSH
- // credentials. Instead, we instantiate a Drupal\Core\FileTransfer\Local
- // and invoke update_authorize_run_update() directly.
- if (fileowner($project_real_location) == fileowner($this->sitePath)) {
+ // Modified for DPL. The file owner check didn't work for us as owners
+ // doesn't match in our case, but the files can be moved just fine.
+ if (TRUE) {
$this->moduleHandler->loadInclude('update', 'inc', 'update.authorize');
$filetransfer = new Local($this->root, \Drupal::service('file_system'));
$response = update_authorize_run_update($filetransfer, $updates);
diff --git a/modules/update/update.manager.inc b/modules/update/update.manager.inc
index 97333a0825..fe5ec5f02f 100644
--- a/modules/update/update.manager.inc
+++ b/modules/update/update.manager.inc
@@ -325,17 +325,14 @@ function update_manager_batch_project_get($project, $url, &$context) {
* @see install_check_requirements()
*/
function update_manager_local_transfers_allowed() {
- $file_system = \Drupal::service('file_system');
- // Compare the owner of a webserver-created temporary file to the owner of
- // the configuration directory to determine if local transfers will be
- // allowed.
- $temporary_file = \Drupal::service('file_system')->tempnam('temporary://', 'update_');
- $site_path = \Drupal::getContainer()->getParameter('site.path');
- $local_transfers_allowed = fileowner($temporary_file) === fileowner($site_path);
-
- // Clean up. If this fails, we can ignore it (since this is just a temporary
- // file anyway).
- @$file_system->unlink($temporary_file);
-
- return $local_transfers_allowed;
+ // Patch for DPL. Normally this checks if a temporary file created is owned by
+ // the same user as `sites/<current site>`, but that's pretty simplistic and
+ // will fail in quite valid cases (for instance if the webserver has write
+ // access through group permissions). In our case, we know that uploaded
+ // modules can be moved to our `sites/default/files/modules_local` (which is
+ // symlinked from `modules/`), but we can't really fix this properly as this
+ // function doesn't know whether it's a module, theme or something else
+ // entirely that we're checking for. So rather than trying to cook up some
+ // generic solution that'll fail in some cases anyway, we just return true.
+ return TRUE;
}

0 comments on commit f5e7071

Please sign in to comment.