Resolve relative symlink paths correctly #1009

fredrikekelund · 2025-03-05T11:19:44Z

Related issues

Fixes Studio Site stopped working after software update - Failed to start the site server #1000 and Error "Could not create directory '../acorn/bin/acorn': Invalid argument" when starting site (works fine in v1.3.4) #1006

Proposed Changes

First, a quick recap of how SymlinkManager works. Symlinks are technically preserved when mounting the site directory in Playground's virtual file system. However, symlinks are in reality just a special type of file with a target path, and if the target path is outside the document root, it's not accessible in Playground's FS. This is where SymlinkManager comes in – it creates a directory inside Playground's FS at the target path and mounts the target path from the host filesystem there.

This PR specifically fixes an issue that was introduced when we recently upgraded our Playground dependencies (#959). After the upgrade, the php.readlink function started returning the correct value, which is the symlink's exact, unresolved target path (i.e. it suddenly returned relative paths sometimes).

This PR changes the SymlinkManager::addSymlink method to resolve the path returned from php.readlink relative to the directory where the symlink lives (i.e. how symlinks are normally handled).

I've also added a SymlinkManager::isTargetInsideDocumentRoot method that allows us to skip mounting symlink target paths that are already accessible inside the document root. Should make SymlinkManager more efficient.After working on fixing the failing tests, I realized that SymlinkManager::mountTarget already has this performance optimization in that it checks if the file or directory already exists in the Playground FS before attempting to mount it.

Testing Instructions

First, let's test the specific case that was reported to us:

Have a Studio site
Download this plugin and unpack it to wp-content/plugins
Run npm install inside the plugin directory
Start the Studio site and ensure that it doesn't encounter any errors

Secondly, let's test a typical symlink targeting a directory outside the document root:

Have a Studio site where Yoast SEO is not already installed
Download Yoast SEO and extract it to ~/wordpress-seo
In your site's wp-content/plugins directory, run ln -s ~/wordpress-seo
Start the Studio site
Navigate to /wp-admin/plugins.php
Ensure that the Yoast SEO plugin is available and that you can activate it without error

Pre-merge Checklist

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

fredrikekelund · 2025-03-05T11:21:08Z

src/lib/symlink-manager.ts

 		);
+		// For some reason, we must access the target directory after mounting it, otherwise the
+		// mount will not work.
+		this.php.listFiles( vfsTarget );


@bgrgicak @adamziel @ashfame, I'm not sure why this is needed, but the Yoast SEO test case (see PR description) didn't work without it in my testing. Any idea why that's the case..?

It continues to work for me when I comment out this line. Maybe it depends on the underlying filesystem and we use different ones?

To quickly double-check: did you restart the entire app after commenting out this line?

I was able to mount and start a site with wp-data-layer-app-pages plugin without listing the files.

Yeah, the wp-data-layer-app-pages test case works without this line for me, too. It's only the Yoast test case that was troublesome

We may find that this line is unnecessary after further testing, but I have a hard time seeing how it would break anything, so I'll land the PR with this included and we can keep the discussion going after merging.

fredrikekelund · 2025-03-05T11:24:26Z

src/lib/symlink-manager.ts


 		// Get the realpath of the symlink within the PHP runtime.
-		const vfsTarget = this.php.readlink( vfsPath );
+		const vfsTarget = path.posix.resolve( path.dirname( vfsPath ), this.php.readlink( vfsPath ) );


php.realpath does not accomplish the same thing (confirmed in my testing). I believe it just returns false because the target path does not yet exist at this point.

To expand on this: realpath works as a more compact way of expressing this logic if the target path is already within the document root. If this is not the case, realpath returns false because the target does not yet exist (see PHP docs).

The SymlinkManager::isTargetInsideDocumentRoot added in this PR stems from the realization that if the target path is already within the document root, there's no need for SymlinkManager to intervene. This is a performance optimization that should help speed up server starts for sites with many symlinks.

Following up about SymlinkManager::isTargetInsideDocumentRoot: I realized that SymlinkManager::mountTarget already has this performance optimization in that it checks if the file or directory already exists in the Playground FS before attempting to mount it. Hence, I removed this method.

wojtekn · 2025-03-05T11:52:28Z

It fixes both cases for me.

sejas

Solves the issue and I can start the site without any ../acorn/bin/acorn symlink error. 👍

sejas · 2025-03-05T12:41:41Z

src/lib/symlink-manager.ts

 		);
+		// For some reason, we must access the target directory after mounting it, otherwise the
+		// mount will not work.
+		this.php.listFiles( vfsTarget );


I was able to mount and start a site with wp-data-layer-app-pages plugin without listing the files.

ivan-ottinger

The changes test well. I haven't been able to reproduce the issue anymore. 👍🏼

Symlinked plugin works well as well.

I did not observe any regressions.

adamziel · 2025-03-05T17:47:11Z

Does that solve for Studio problems with symlinks?

fredrikekelund · 2025-03-06T09:39:16Z

@adamziel, not entirely. See https://github.com/Automattic/dotcom-forge/issues/10566 and #548. I'm anticipating that we'll need some assistance with Windows support – the issue description should give you a decent idea of what the problem is currently.

fredrikekelund added 3 commits March 5, 2025 11:47

Resolve relative symlink paths correctly

8c66854

Skip mounting symlinks where target is already inside document root

65eae59

Fix

a77c724

fredrikekelund requested review from a team and bgrgicak March 5, 2025 11:19

fredrikekelund self-assigned this Mar 5, 2025

fredrikekelund commented Mar 5, 2025

View reviewed changes

fredrikekelund linked an issue Mar 5, 2025 that may be closed by this pull request

Error "Could not create directory '../acorn/bin/acorn': Invalid argument" when starting site (works fine in v1.3.4) #1006

Closed

Fix tests and remove isTargetInsideDocumentRoot

a81c93f

wojtekn approved these changes Mar 5, 2025

View reviewed changes

sejas approved these changes Mar 5, 2025

View reviewed changes

ivan-ottinger self-requested a review March 5, 2025 12:44

ivan-ottinger approved these changes Mar 5, 2025

View reviewed changes

fredrikekelund merged commit 85b1ced into trunk Mar 5, 2025
8 checks passed

fredrikekelund deleted the f26d/fix-relative-symlink-targets branch March 5, 2025 12:47

Resolve relative symlink paths correctly #1009

Resolve relative symlink paths correctly #1009

Uh oh!

Conversation

fredrikekelund commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related issues

Proposed Changes

Testing Instructions

Pre-merge Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wojtekn commented Mar 5, 2025

Uh oh!

sejas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivan-ottinger left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adamziel commented Mar 5, 2025

Uh oh!

fredrikekelund commented Mar 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

fredrikekelund commented Mar 5, 2025 •

edited

Loading

ivan-ottinger left a comment •

edited

Loading