fix: use Lstat in PurgeDirectory to handle symlinks without following them#8254
Merged
Conversation
… them [skip buildkite] PurgeDirectory and PurgeDirectoryExcept used os.Stat (via util.Chmod) which follows symlinks. If a symlink's target was already removed earlier in the loop, the stat would fail with ENOENT. Use os.Lstat instead and skip chmod for symlinks — symlinks have no meaningful permissions of their own and os.RemoveAll removes the symlink entry itself without following it. This surfaced in TestDdevImportFilesDir for TYPO3: the v13.4.19 files.tgz contains test.txt as a symlink to README.txt. After #8213 added TypeSymlink handling to Untar, symlinks are now extracted to the host. When README.txt was removed before test.txt in the purge loop, test.txt became a dangling symlink and the subsequent util.Chmod failed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Download the artifacts for this pull request:
See Testing a PR.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Issue
A symlink in the TYPO3 v13.4.19
files.tgz(test.txt -> README.txt) caused intermittent failures inTestDdevImportFilesDiron CI: failing runHow This PR Solves The Issue
PurgeDirectory(andPurgeDirectoryExcept) iterated directory entries and calledutil.Chmodbefore removing each entry.util.Chmodusesos.Stat, which follows symlinks. If the symlink's target had already been removed earlier in the loop,os.Statwould fail withENOENT.This PR switches to
os.Lstatto inspect each entry without following symlinks. Symlinks are then removed directly viaos.RemoveAll(which removes the symlink entry itself, not its target) without any chmod step — symlinks don't have meaningful permissions of their own.Relationship to the ZipSlip fix (#8213)
PR #8213 added
case tar.TypeSymlinkhandling toUntar, so symlinks in archives are now properly extracted to the host filesystem. Before that PR, thetest.txtsymlink entry in TYPO3'sfiles.tgzwas silently skipped during extraction and never appeared on the host, soPurgeDirectorynever encountered it.After #8213,
test.txtis extracted as a real symlink (test.txt -> README.txt). WhenPurgeDirectorylater iteratesfileadmin,Readdirnamesreturns entries in arbitrary filesystem/inode order. IfREADME.txtis encountered first and removed,test.txtbecomes a dangling symlink — and the subsequentutil.Chmodcall on it fails.Why it was intermittent
Readdirnamesreturns entries in inode order, which varies by runner and filesystem. The failure only occurs whenREADME.txtsorts beforetest.txtin that order, making the bug dependent on the specific runner environment.Manual Testing Instructions
ddev import-filesagainst a TYPO3 project that has symlinks in itsfileadmin/directory and confirm the import completes without errorarchive.Untarpath is unchanged)TestDdevImportFilesDirCI failure for TYPO3 should be resolved; watch the next CI runs to confirmAutomated Testing Overview
Existing
TestPurgeDirectoryandTestPurgeDirectoryExcepttests pass. The intermittent CI failure inTestDdevImportFilesDirfor TYPO3 will need to be observed over subsequent CI runs to confirm the fix holds.Release/Deployment Notes
No impact on behavior for non-symlink files. Symlinks in upload directories are now removed cleanly during
ddev import-filespre-import cleanup rather than causing an error.