Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override default umask in PHP #555

Merged
merged 3 commits into from
Sep 24, 2024
Merged

Override default umask in PHP #555

merged 3 commits into from
Sep 24, 2024

Conversation

fluiddot
Copy link
Contributor

@fluiddot fluiddot commented Sep 23, 2024

Related to 8816-gh-Automattic/dotcom-forge.

Proposed Changes

  • The original issue needs to be fixed in Emscripten. There's already a PR to address it but it will take time until the changes will be incorporated in the app, as we'll need to wait for new Emscripten and Playground versions.
  • In the meantime, we'll create an internal plugin that will override the default umask value. Note that this plugin is located in a special folder managed by Playground for platform-level-specific plugins.
  • As a temporary workaround, we'll override the default umask value by pretending a PHP file using the auto_prepend_file PHP directive.

Testing Instructions

Create files with test file

  • Run the app with the command npm start.
  • Create a new site or select one already created.
  • Create the file test.php, at the root level on the site, with the following content:
<?php
// Create file with default umask
$filename = "example-default-umask.txt";
$current_umask = umask();
    echo sprintf("Current umask: %04o<br>", $current_umask);
    $content = 'This is the content of the file.';
if (file_exists($filename)) {
    unlink($filename);
}
if (file_put_contents($filename, $content) !== false) {
    echo "File '$filename' created successfully.<br>";
} else {
    echo "Error creating file '$filename'.<br>";
}
chmod($filename, 0777 - umask());
  • Start the site.
  • Navigate to the PHP file: http://localhost:<PORT>/test.php.
  • Observe the umask value shown on the page is 0022.
  • Navigate to the site folder.
  • Observe that the permissions of the file are rwxr-xr-x (Full access to owner, and only READ and EXECUTE permissions to group and others).

Addresses #269

  • Start the app with command npm start.
  • Create a new site.
  • Change the PHP version to 8.2.
  • Click on the Terminal button.
  • In the terminal:
    • Run cd wp-content/themes.
    • Run git clone https://github.com/DevArge/sage-vite.git.
    • Run cd sage-vite.
    • Run yarn install && composer install && yarn build.
  • Back in the Studio app, start the site.
  • Navigate to WP-Admin dashboard.
  • Navigate to Appearance -> Themes.
  • Activate the Sage Vite theme.
  • Observe that no errors are thrown and the site is working as expected.

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

@fluiddot fluiddot requested a review from a team September 23, 2024 15:02
@fluiddot fluiddot self-assigned this Sep 23, 2024
@fluiddot fluiddot changed the title Set default umask in PHP more permissive Override default umask in PHP Sep 23, 2024
vendor/wp-now/src/wp-now.ts Outdated Show resolved Hide resolved
vendor/wp-now/src/wp-now.ts Outdated Show resolved Hide resolved
vendor/wp-now/src/wp-now.ts Outdated Show resolved Hide resolved
@fluiddot
Copy link
Contributor Author

Heads up that this solution won't fix 8818-gh-Automattic/dotcom-forge. More info in https://github.com/Automattic/dotcom-forge/issues/8818#issuecomment-2368833340.

@fluiddot
Copy link
Contributor Author

Following this comment, I'm going to implement a different approach to override the umask value. Until it's ready, I'll change the PR to draft 🔧 .

@fluiddot fluiddot marked this pull request as draft September 24, 2024 08:18
This approach replaces the one using a plugin.
@fluiddot fluiddot marked this pull request as ready for review September 24, 2024 09:04
@fluiddot fluiddot linked an issue Sep 24, 2024 that may be closed by this pull request
Copy link
Contributor

@wojtekn wojtekn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fixes the issue for me. I was able to fix the broken Sage site:

rm -rf wp-content/cache
cd wp-content/themes/sage-vite 
yarn install && composer install && yarn build

@fluiddot fluiddot merged commit 8cb7361 into trunk Sep 24, 2024
11 checks passed
@fluiddot fluiddot deleted the fix/php-default-umask branch September 24, 2024 10:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Permission issues using a theme that uses composer
2 participants