diff --git a/core/includes/install.inc b/core/includes/install.inc index 6e6db7aa4eeade91a76c34fad417ca04dc2c0afe..56b450ec95c150aafacaa306b77e4ffd0e7b6c25 100644 --- a/core/includes/install.inc +++ b/core/includes/install.inc @@ -660,11 +660,14 @@ function drupal_install_system($install_state) { * An optional bitmask created from various FILE_* constants. * @param $type * The type of file. Can be file (default), dir, or link. + * @param bool $autofix + * (optional) Determines whether to attempt fixing the permissions according + * to the provided $mask. Defaults to TRUE. * * @return * TRUE on success or FALSE on failure. A message is set for the latter. */ -function drupal_verify_install_file($file, $mask = NULL, $type = 'file') { +function drupal_verify_install_file($file, $mask = NULL, $type = 'file', $autofix = TRUE) { $return = TRUE; // Check for files that shouldn't be there. if (isset($mask) && ($mask & FILE_NOT_EXIST) && file_exists($file)) { @@ -686,7 +689,7 @@ function drupal_verify_install_file($file, $mask = NULL, $type = 'file') { switch ($current_mask) { case FILE_EXIST: if (!file_exists($file)) { - if ($type == 'dir') { + if ($type == 'dir' && $autofix) { drupal_install_mkdir($file, $mask); } if (!file_exists($file)) { @@ -695,32 +698,32 @@ function drupal_verify_install_file($file, $mask = NULL, $type = 'file') { } break; case FILE_READABLE: - if (!is_readable($file) && !drupal_install_fix_file($file, $mask)) { + if (!is_readable($file)) { $return = FALSE; } break; case FILE_WRITABLE: - if (!is_writable($file) && !drupal_install_fix_file($file, $mask)) { + if (!is_writable($file)) { $return = FALSE; } break; case FILE_EXECUTABLE: - if (!is_executable($file) && !drupal_install_fix_file($file, $mask)) { + if (!is_executable($file)) { $return = FALSE; } break; case FILE_NOT_READABLE: - if (is_readable($file) && !drupal_install_fix_file($file, $mask)) { + if (is_readable($file)) { $return = FALSE; } break; case FILE_NOT_WRITABLE: - if (is_writable($file) && !drupal_install_fix_file($file, $mask)) { + if (is_writable($file)) { $return = FALSE; } break; case FILE_NOT_EXECUTABLE: - if (is_executable($file) && !drupal_install_fix_file($file, $mask)) { + if (is_executable($file)) { $return = FALSE; } break; @@ -728,6 +731,9 @@ function drupal_verify_install_file($file, $mask = NULL, $type = 'file') { } } } + if (!$return && $autofix) { + return drupal_install_fix_file($file, $mask); + } return $return; } diff --git a/core/modules/system/system.install b/core/modules/system/system.install index 5b2e4b9950b94a0cf926619e7a5c4e78f83a6ce9..3ad360ff242e54b22772e83196f7e581bdc1db50 100644 --- a/core/modules/system/system.install +++ b/core/modules/system/system.install @@ -436,12 +436,13 @@ function system_requirements($phase) { // directory. This allows additional files in the site directory to be // updated when they are managed in a version control system. if (Settings::get('skip_permissions_hardening')) { - $conf_errors[] = t('Protection disabled'); + $error_value = t('Protection disabled'); // If permissions hardening is disabled, then only show a warning for a // writable file, as a reminder, rather than an error. $file_protection_severity = REQUIREMENT_WARNING; } else { + $error_value = t('Not protected'); // In normal operation, writable files or directories are an error. $file_protection_severity = REQUIREMENT_ERROR; if (!drupal_verify_install_file($site_path, FILE_NOT_WRITABLE, 'dir')) { @@ -450,7 +451,7 @@ function system_requirements($phase) { } foreach (['settings.php', 'settings.local.php', 'services.yml'] as $conf_file) { $full_path = $site_path . '/' . $conf_file; - if (file_exists($full_path) && (Settings::get('skip_permissions_hardening') || !drupal_verify_install_file($full_path, FILE_EXIST | FILE_READABLE | FILE_NOT_WRITABLE))) { + if (file_exists($full_path) && !drupal_verify_install_file($full_path, FILE_EXIST | FILE_READABLE | FILE_NOT_WRITABLE, 'file', !Settings::get('skip_permissions_hardening'))) { $conf_errors[] = t("The file %file is not protected from modifications and poses a security risk. You must change the file's permissions to be non-writable.", ['%file' => $full_path]); } } @@ -472,7 +473,7 @@ function system_requirements($phase) { ]; } $requirements['configuration_files'] = [ - 'value' => t('Not protected'), + 'value' => $error_value, 'severity' => $file_protection_severity, 'description' => $description, ]; diff --git a/core/modules/system/tests/src/Functional/System/SitesDirectoryHardeningTest.php b/core/modules/system/tests/src/Functional/System/SitesDirectoryHardeningTest.php index fbe97f4dab8df94465e63c1a5d3c6cab2f06f494..f08a18d8bc4158445eb30e90708857d51553dd99 100644 --- a/core/modules/system/tests/src/Functional/System/SitesDirectoryHardeningTest.php +++ b/core/modules/system/tests/src/Functional/System/SitesDirectoryHardeningTest.php @@ -49,13 +49,16 @@ public function testSitesDirectoryHardeningConfig() { $settings = Settings::getAll(); $settings['skip_permissions_hardening'] = TRUE; new Settings($settings); - $this->assertTrue(Settings::get('skip_permissions_hardening'), 'Able to set hardening to true'); + $this->assertTrue(Settings::get('skip_permissions_hardening'), 'Able to set skip permissions hardening to true.'); $this->makeWritable($site_path); // Manually trigger the requirements check. $requirements = $this->checkSystemRequirements(); $this->assertEqual(REQUIREMENT_WARNING, $requirements['configuration_files']['severity'], 'Warning severity is properly set.'); - $this->assertEqual($this->t('Protection disabled'), (string) $requirements['configuration_files']['description']['#context']['configuration_error_list']['#items'][0], 'Description is properly set.'); + $this->assertEquals('Protection disabled', (string) $requirements['configuration_files']['value']); + $description = strip_tags(\Drupal::service('renderer')->renderPlain($requirements['configuration_files']['description'])); + $this->assertContains('settings.php is not protected from modifications and poses a security risk.', $description); + $this->assertContains('services.yml is not protected from modifications and poses a security risk.', $description); $this->assertTrue(is_writable($site_path), 'Site directory remains writable when automatically fixing permissions is disabled.'); $this->assertTrue(is_writable($settings_file), 'settings.php remains writable when automatically fixing permissions is disabled.'); @@ -66,7 +69,8 @@ public function testSitesDirectoryHardeningConfig() { new Settings($settings); // Manually trigger the requirements check. - $this->checkSystemRequirements(); + $requirements = $this->checkSystemRequirements(); + $this->assertEquals('Protected', (string) $requirements['configuration_files']['value']); $this->assertFalse(is_writable($site_path), 'Site directory is protected when automatically fixing permissions is enabled.'); $this->assertFalse(is_writable($settings_file), 'settings.php is protected when automatically fixing permissions is enabled.');