Skip to content

Commit bbc4546

Browse files
adamzieladamziel
committed
Allow symlink creation through symlinked directories in preserve-local mode
When migrating between two WP Cloud Atomic sites, themes and plugins are managed as symlinks inside shared directories. On the target site, /wordpress/ is itself a symlink to the shared WordPress installation. The preserve-local mode was blanket-blocking all content creation through symlinked directories, which silently prevented importing themes that existed on the source but not on the target. The fix distinguishes symlink entries from regular files: creating a symlink just adds a directory entry (a pointer), it doesn't modify existing shared content. When a symlink's parent directory is writable, the operation is allowed. Read-only parent directories remain fully protected, and regular files through symlinked paths are still blocked.
1 parent accdc70 commit bbc4546

3 files changed

Lines changed: 217 additions & 17 deletions

File tree

‎packages/reprint-importer/src/import.php‎

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5880,7 +5880,7 @@ private function diff_indexes_and_build_fetch_list(): bool
58805880
$local === null ||
58815881
strcmp($local["path"], $remote["path"]) > 0
58825882
) {
5883-
$skip_reason = $this->should_skip_for_preserve_local($remote["path"]);
5883+
$skip_reason = $this->should_skip_for_preserve_local($remote["path"], $remote["type"] ?? "file");
58845884
if ($skip_reason) {
58855885
$this->audit_log($skip_reason, true);
58865886
$this->emit_skip_progress($remote["path"]);
@@ -7937,14 +7937,32 @@ private function display_path(string $path): string
79377937
return $rel;
79387938
}
79397939

7940-
/**
7941-
* Check whether any component of the path (between the filesystem root
7942-
* and the target) is a symlink. In preserve-local mode this is used
7943-
* to prevent creating new content through symlinked directories — their
7944-
* contents belong to shared hosting infrastructure and must not be
7945-
* modified.
7940+
/*
7941+
* Preserve-local and the symlink exemption
7942+
* -----------------------------------------
7943+
* On shared hosts the fs-root often contains symlinked directories
7944+
* that point to infrastructure managed outside the site (e.g. on
7945+
* WP Cloud Atomic, /wordpress/ is a symlink to shared WP core).
7946+
* Writing regular files through those symlinks would land them in
7947+
* the shared tree, so preserve-local blocks that.
7948+
*
7949+
* Symlink *entries* are different: they don't write data into the
7950+
* target directory, they just add a pointer in the parent directory.
7951+
* On Atomic, that's exactly how per-site themes and plugins are
7952+
* installed — a symlink is created in /wordpress/themes/pub/.
7953+
* Blocking that prevents importing themes that only exist on the
7954+
* source site.
7955+
*
7956+
* Rule: when a path traverses a symlink, block regular files but
7957+
* allow creating symlink entries if the parent dir is writable.
7958+
* A read-only parent still blocks everything.
7959+
*
7960+
* This rule is applied in two places:
7961+
* - should_skip_for_preserve_local() (diff / download-list phase)
7962+
* - handle_symlink_chunk() (fetch / disk-write phase)
79467963
*/
7947-
private function should_skip_for_preserve_local(string $path): ?string
7964+
7965+
private function should_skip_for_preserve_local(string $path, string $type = "file"): ?string
79487966
{
79497967
if ($this->fs_root_nonempty_behavior !== 'preserve-local') {
79507968
return null;
@@ -7960,14 +7978,15 @@ private function should_skip_for_preserve_local(string $path): ?string
79607978
return "PRESERVE-LOCAL skip file (exists): {$path}";
79617979
}
79627980

7963-
// Skip if parent directory is not writable or if any directory component
7964-
// in the path is a symlink. We never create new files through symlinks —
7965-
// the symlink and its target contents are shared hosting infrastructure.
79667981
$dir = dirname($local_path);
79677982
if (is_dir($dir) && !is_writable($dir)) {
79687983
return "PRESERVE-LOCAL skip file (dir not writable): {$path}";
79697984
}
79707985
if ($this->path_traverses_symlink($dir)) {
7986+
// Symlink exemption — see block comment above.
7987+
if ($type === "link" && is_dir($dir) && is_writable($dir)) {
7988+
return null;
7989+
}
79717990
return "PRESERVE-LOCAL skip file (symlink in path): {$path}";
79727991
}
79737992

@@ -8246,18 +8265,21 @@ private function handle_symlink_chunk(array $chunk): void
82468265

82478266
// In preserve-local mode, if something already exists at the symlink
82488267
// path, keep it — whether it's a file, directory, or another symlink.
8249-
// Also skip if any parent component is a symlink — we never create
8250-
// new content through symlinked directories.
82518268
if ($this->fs_root_nonempty_behavior === 'preserve-local') {
82528269
if (file_exists($local_path) || is_link($local_path)) {
82538270
$this->audit_log("PRESERVE-LOCAL skip symlink (path exists): {$path} -> {$target}", true);
82548271
$this->emit_skip_progress($path);
82558272
return;
82568273
}
8257-
if ($this->path_traverses_symlink(dirname($local_path))) {
8258-
$this->audit_log("PRESERVE-LOCAL skip symlink (symlink in path): {$path} -> {$target}", true);
8259-
$this->emit_skip_progress($path);
8260-
return;
8274+
$parent_dir = dirname($local_path);
8275+
if ($this->path_traverses_symlink($parent_dir)) {
8276+
// Symlink exemption — see block comment above should_skip_for_preserve_local().
8277+
if (!is_dir($parent_dir) || !is_writable($parent_dir)) {
8278+
$this->audit_log("PRESERVE-LOCAL skip symlink (symlink in path, parent not writable): {$path} -> {$target}", true);
8279+
$this->emit_skip_progress($path);
8280+
return;
8281+
}
8282+
$this->audit_log("PRESERVE-LOCAL allow symlink through symlinked directory: {$path} -> {$target}", false);
82618283
}
82628284
}
82638285

‎tests/Import/FilesSyncStateTest.php‎

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,4 +368,88 @@ public function testFetchStageOverwritesPreviouslySyncedFile()
368368
"Fetch stage must overwrite existing files that were placed in the download list",
369369
);
370370
}
371+
372+
// ---------------------------------------------------------------
373+
// Symlink-through-symlink diff tests (WP Cloud Atomic migration)
374+
// ---------------------------------------------------------------
375+
376+
/**
377+
* In preserve-local mode, a symlink entry whose path traverses a
378+
* symlinked directory should be added to the download list when the
379+
* resolved parent directory is writable.
380+
*
381+
* This reproduces the WP Cloud Atomic bug: /wordpress/ is a symlink
382+
* on the target, and /wordpress/themes/pub/indice is a theme symlink
383+
* that only exists on the source. The diff phase was skipping it
384+
* because path_traverses_symlink() returned true, even though the
385+
* parent directory was writable and the operation is safe.
386+
*/
387+
public function testDeltaDiffIncludesSymlinkThroughSymlinkedWritableDir()
388+
{
389+
// Build a WP Cloud-like target: wordpress/ is a symlink to a
390+
// real directory with a writable themes/pub/ inside.
391+
mkdir($this->fs_root . '/wp-shared/themes/pub', 0755, true);
392+
symlink('wp-shared', $this->fs_root . '/wordpress');
393+
394+
// Local index: empty (the symlink was never synced)
395+
$localIndex = $this->stateDir . '/.import-index.jsonl';
396+
file_put_contents($localIndex, '');
397+
398+
// Remote index: the theme symlink exists on the source
399+
$remoteIndex = $this->stateDir . '/.import-remote-index.jsonl';
400+
file_put_contents($remoteIndex, $this->indexLine('/wordpress/themes/pub/indice', 1000, 0, 'link'));
401+
402+
$this->writeState([
403+
"command" => "files-pull",
404+
"status" => "in_progress",
405+
"stage" => "diff",
406+
]);
407+
408+
[$client, $reflection] = $this->prepareClient();
409+
410+
$diffMethod = $reflection->getMethod('diff_indexes_and_build_fetch_list');
411+
$diffMethod->invoke($client);
412+
413+
$downloads = $this->readDownloadList();
414+
$this->assertContains(
415+
'/wordpress/themes/pub/indice',
416+
$downloads,
417+
"A symlink entry through a symlinked writable directory must be added to the download list",
418+
);
419+
}
420+
421+
/**
422+
* Regular files through symlinked directories should still be
423+
* skipped in preserve-local mode — only symlinks get the exemption.
424+
*/
425+
public function testDeltaDiffStillSkipsFilesThroughSymlinkedDir()
426+
{
427+
mkdir($this->fs_root . '/wp-shared/themes/pub', 0755, true);
428+
symlink('wp-shared', $this->fs_root . '/wordpress');
429+
430+
$localIndex = $this->stateDir . '/.import-index.jsonl';
431+
file_put_contents($localIndex, '');
432+
433+
// A regular file (not a symlink) through the symlinked path
434+
$remoteIndex = $this->stateDir . '/.import-remote-index.jsonl';
435+
file_put_contents($remoteIndex, $this->indexLine('/wordpress/themes/pub/indice/style.css', 1000, 500, 'file'));
436+
437+
$this->writeState([
438+
"command" => "files-pull",
439+
"status" => "in_progress",
440+
"stage" => "diff",
441+
]);
442+
443+
[$client, $reflection] = $this->prepareClient();
444+
445+
$diffMethod = $reflection->getMethod('diff_indexes_and_build_fetch_list');
446+
$diffMethod->invoke($client);
447+
448+
$downloads = $this->readDownloadList();
449+
$this->assertNotContains(
450+
'/wordpress/themes/pub/indice/style.css',
451+
$downloads,
452+
"Regular files through symlinked directories must still be skipped by preserve-local",
453+
);
454+
}
371455
}

‎tests/Import/ImportSymlinkTest.php‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,4 +289,98 @@ public function testSymlinkReplacesExistingFile()
289289
$this->assertTrue(is_link($filePath), 'File should be replaced with symlink');
290290
$this->assertEquals('target', readlink($filePath));
291291
}
292+
293+
/**
294+
* In preserve-local mode, creating a symlink through a symlinked
295+
* directory should succeed when the parent directory is writable.
296+
*
297+
* This reproduces the WP Cloud Atomic migration bug: themes are
298+
* symlinks under /wordpress/themes/pub/, but /wordpress/ itself is a
299+
* symlink on the target. The old code blocked ALL symlink creation
300+
* through symlinked directories, which prevented importing themes
301+
* that only existed on the source site.
302+
*/
303+
public function testPreserveLocalAllowsSymlinkThroughSymlinkedWritableDir()
304+
{
305+
$fsRoot = $this->tempDir . '/fs-root';
306+
307+
// Build a structure that mirrors WP Cloud Atomic:
308+
// fs-root/wp-shared/themes/pub/ (real, writable directory)
309+
// fs-root/wordpress -> wp-shared (symlink)
310+
mkdir($fsRoot . '/wp-shared/themes/pub', 0755, true);
311+
symlink('wp-shared', $fsRoot . '/wordpress');
312+
313+
$client = new \ImportClient('http://fake.url', $this->tempDir, $fsRoot);
314+
315+
$reflection = new \ReflectionClass($client);
316+
317+
// Enable preserve-local mode
318+
$behaviorProp = $reflection->getProperty('fs_root_nonempty_behavior');
319+
$behaviorProp->setValue($client, 'preserve-local');
320+
321+
$method = $reflection->getMethod('handle_symlink_chunk');
322+
323+
// Try to create a theme symlink through the symlinked /wordpress/ path
324+
$chunk = [
325+
'headers' => [
326+
'x-symlink-path' => base64_encode('/wordpress/themes/pub/indice'),
327+
'x-symlink-target' => base64_encode('indice-1.0'),
328+
'x-symlink-ctime' => '1234567890'
329+
]
330+
];
331+
332+
$method->invoke($client, $chunk);
333+
334+
$symlinkPath = $fsRoot . '/wordpress/themes/pub/indice';
335+
$this->assertTrue(
336+
is_link($symlinkPath),
337+
'Symlink should be created through a symlinked directory when the parent is writable'
338+
);
339+
$this->assertEquals('indice-1.0', readlink($symlinkPath));
340+
}
341+
342+
/**
343+
* In preserve-local mode, creating a symlink through a symlinked
344+
* read-only directory should still be blocked. This protects shared
345+
* hosting infrastructure that is genuinely read-only.
346+
*/
347+
public function testPreserveLocalBlocksSymlinkThroughSymlinkedReadOnlyDir()
348+
{
349+
$fsRoot = $this->tempDir . '/fs-root';
350+
351+
// Build a structure with a read-only shared directory:
352+
// fs-root/wp-shared/themes/pub/ (real, read-only directory)
353+
// fs-root/wordpress -> wp-shared (symlink)
354+
mkdir($fsRoot . '/wp-shared/themes/pub', 0755, true);
355+
symlink('wp-shared', $fsRoot . '/wordpress');
356+
chmod($fsRoot . '/wp-shared/themes/pub', 0555);
357+
358+
$client = new \ImportClient('http://fake.url', $this->tempDir, $fsRoot);
359+
360+
$reflection = new \ReflectionClass($client);
361+
362+
$behaviorProp = $reflection->getProperty('fs_root_nonempty_behavior');
363+
$behaviorProp->setValue($client, 'preserve-local');
364+
365+
$method = $reflection->getMethod('handle_symlink_chunk');
366+
367+
$chunk = [
368+
'headers' => [
369+
'x-symlink-path' => base64_encode('/wordpress/themes/pub/indice'),
370+
'x-symlink-target' => base64_encode('indice-1.0'),
371+
'x-symlink-ctime' => '1234567890'
372+
]
373+
];
374+
375+
$method->invoke($client, $chunk);
376+
377+
// Restore writability for cleanup
378+
chmod($fsRoot . '/wp-shared/themes/pub', 0755);
379+
380+
$symlinkPath = $fsRoot . '/wordpress/themes/pub/indice';
381+
$this->assertFalse(
382+
is_link($symlinkPath),
383+
'Symlink should NOT be created through a symlinked directory when the parent is read-only'
384+
);
385+
}
292386
}

0 commit comments

Comments
 (0)