Skip to content

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
@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 requested a review from wojtekn 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
@fluiddot fluiddot deleted the fix/php-default-umask branch September 24, 2024 10:10
@lajennylove
Copy link

Hey there guys!

I was just wondering (because I'm seen this is already merged) when is this going to be available in production studio? Because I opened this ticket #269 (comment) and it said it was going to be taken care over here. Nevertheless even having the latest Studio version, with PHP 8.3

Screenshot 2024-11-11 at 10 31 33 a m

This is still happening.
Screenshot 2024-11-11 at 10 32 28 a m

Best regards.

@wojtekn
Copy link
Contributor

wojtekn commented Nov 11, 2024

@lajennylove thanks for retesting it. We fixed the permissions issue, so the directory should have the correct ones when it's created, but if your directory was created before, it will likely still have incorrect permissions.

Please try removing that cache directory and trying again.

@lajennylove
Copy link

Hey @wojtekn!

I just tested it as you mentioned and it completely worked.

Screenshot 2024-11-11 at 1 27 59 p m

I will try with a real project that I'm working with it to check its relability, and if it works I will implemented it in my company as part of our local development stack.

Just a quick question (I know this is not the channel maybe and I have to raise other ticket but) I was just wondering if is there a way to control the .htaccess/nginx rules to try something like this?

<IfModule mod_rewrite.c>
RewriteEngine On
RewriteRule ^wp-content/uploads/(.*)$ https://www.domain.com/wp-content/uploads/$1 [R=301,L]
</IfModule>

Best regards.

@wojtekn
Copy link
Contributor

wojtekn commented Nov 19, 2024

@lajennylove thanks for confirming that.

Just a quick question (I know this is not the channel maybe and I have to raise other ticket but) I was just wondering if is there a way to control the .htaccess/nginx rules to try something like this?

No, Studio currently doesn't support Apache-like rewrite rules.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Permission issues using a theme that uses composer

4 participants