Fatal error when PHPCS tries to create a `LocalFile` object for a symlinked directory on Windows

[jrfnl]

Summary

Came across this by chance. Documenting the error here as this needs further investigation.

Looks like PHPCS tries to create a LocalFile object for symlinked directories on Windows, which results in a Failed to open stream: Permission denied in path/to/PHP_CodeSniffer/src/Files/LocalFile.php error.

To my surprise, it also looks like this is related to a change in PHP itself as the issue is not reproducable on PHP < 8.1 and consistently reproducible on PHP 8.1+.

Detailed outline of the problem

Given a composer.json like this (minimal example based on Symfony - take note of the repositories entries using "type": "path"):

{
    "type": "library",
    "require": {
        "symfony/contracts": "^3.6",
    },
    "require-dev": {
        "symfony/runtime": "self.version",
    },
    "repositories": [
        {
            "type": "path",
            "url": "src/Symfony/Contracts",
            "options": {
                "versions": {
                    "symfony/contracts": "3.6.x-dev"
                }
            }
        },
        {
            "type": "path",
            "url": "src/Symfony/Component/Runtime"
        }
    ],
    "minimum-stability": "dev"
}

... Composer will create symlinked directories for the symfony/contracts and symfony/runtime dependencies in the vendor directory. On Windows, I'm observing that it also copies over the files, but that's irrelevant for the current issue.

On Windows, that symlinked directory path will identify as readable, but opening the "file" will result in a Failed to open stream: Permission denied warning, which is then caught by PHPCS and turned into a fatal error by the PHPCS error handler in the Runner class.

This is the error I've seen:

Fatal error: Uncaught PHP_CodeSniffer\Exceptions\RuntimeException: fopen(path/to/symfony/vendor/symfony/contracts): Failed to open stream: Permission denied in path/to/PHP_CodeSniffer/src/Files/LocalFile.php on line 47 in path/to/PHP_CodeSniffer/src/Runner.php:543
Stack trace:
#0 [internal function]: PHP_CodeSniffer\Runner->handleErrors(2, 'fopen(path/to/...', 'path/to/...', 47)
#1 path/to/PHP_CodeSniffer/src/Files/LocalFile.php(47): fopen('path/to/...', 'r')
#2 path/to/PHP_CodeSniffer/src/Files/FileList.php(201): PHP_CodeSniffer\Files\LocalFile->__construct('path/to/...', Object(PHP_CodeSniffer\Ruleset), Object(PHP_CodeSniffer\Config))
#3 path/to/PHP_CodeSniffer/src/Runner.php(379): PHP_CodeSniffer\Files\FileList->current()
#4 path/to/PHP_CodeSniffer/src/Runner.php(122): PHP_CodeSniffer\Runner->run()
#5 path/to/PHP_CodeSniffer/bin/phpcs(30): PHP_CodeSniffer\Runner->runPHPCS()
#6 {main}
  thrown in path/to/PHP_CodeSniffer/src/Runner.php on line 543

To reproduce

• Clone the Symfony repo [email protected]:symfony/symfony.git
• Run composer install
• Using a local clone of PHPCS on PHP < 8.1, run phpcs -ps ./vendor/symfony/ --standard=generic --report=summary --extensions=php --sniffs=generic.files.byteordermark and observe no errors.
• Switch to PHP 8.1 or higher.
• Run the same PHPCS command again and observe the error.

Analysis

The error occurs when PHPCS tries to create a LocalFile object for the symlinked directory, but PHPCS shouldn't create File objects for directories to begin with.

Changing line https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/0ca86845ce43291e8f5692c7356fccf3bcf02bf4/src/Files/LocalFile.php#L33 like so:

-        if (Common::isReadable($this->path) === false) {
+        if (Common::isReadable($this->path) === false || @is_file($this->path) === false) {

gets rid of the error, but might have side-effects.

Changing Common::isReadable() to only apply to files is not a viable solution direction, as the method is also used in the Config class to verify if directories are readable when that class is searching for a ruleset file. https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/0ca86845ce43291e8f5692c7356fccf3bcf02bf4/src/Config.php#L429

Moreover, the FileList iterator class should probably only contain file references and discard directories, which should prevent the problem from occurring to begin with. However, if the class wasn't set up like that from the start, changing this may again have side-effects.

Next steps

• Some digging needs to be done through both the FileList class code as well as through its history to see why the symlinked directory is included in the FileList iterator and whether that is correct or not.
• Some digging needs to be done to find out what change from PHP 8.1 is causing the issue and what the typical solution is to deal with that PHP 8.1 change.

That research should inform the solution for the issue.

This also needs a good think about how we can safeguard any fix for this issue via tests. Note: this part of the codebase is (again) barely covered by tests, so while tests just covering this issue would be a good starting point, more comprehensive tests would be better.

Other open questions

• Does this issue only occur on Windows or also on Linux ?
• Is this error specifically related to how these symlinks are created by Composer ? Or does this apply to all symlinked directories on Windows ?

Versions (please complete the following information)

Operating System	Windows 10
PHP version	PHP >= 8.1
PHP_CodeSniffer version	3.x + 4.x, I've been able to reproduce with 3.5.0 and haven't tried further back
Standard	not relevant
Install type	not relevant

Please confirm

• [x] I have searched the issue list and am not opening a duplicate issue.
• [x] I have read the Contribution Guidelines and this is not a support question.
• [x] I confirm that this bug is a bug in PHP_CodeSniffer and not in one of the external standards.
• [x] I have verified the issue still exists in the 4.x branch of PHP_CodeSniffer.

[jrfnl]

Previous issue which may be loosely related:

• https://github.com/squizlabs/PHP_CodeSniffer/issues/2965 which was fixed via https://github.com/squizlabs/PHP_CodeSniffer/commit/a3e045313cb7651b587acca17e190cbfd0c81fae

[jrfnl]

Also loosely related:

PHPCS records an "Error opening file; file no longer exists or you do not have access to read the file" error when an unreadable file is encountered.

https://github.com/PHPCSStandards/PHP_CodeSniffer/blob/2c13a9078eb3a680dfbda085eb90e6c6bb05a023/src/Files/LocalFile.php#L37-L39

However, that error doesn't seem to be displayed to the end-user (based on the error not displaying with the diff shown under "Analysis" applied).

This may need looking into when this issue is being addressed. Or this may need its own issue. At the very least, comprehensive tests will need to be added to figure out if and if so, under what circumstances the error is currently being displayed and which cases need fixing.