Skip to content

Proposal to enable missingCheckedExceptionInThrows phpstan rule in pm6 #6762

@DaisukeDaisuke

Description

@DaisukeDaisuke

Problem description

To prevent external leaks, the use of RuntimeException on the main thread is now discouraged in pm6

In PM, \RuntimeException is still used frequently in events and similar contexts
Banning \RuntimeException and introducing a PHPStan check would help prevent supply chain attacks, such as the one documented below:
GHSA-8cwq-4cmf-px73
However, fully banning RuntimeException may create technical debt and could interfere with the addition of new features in the future

This proposal is tentative and subject to discussion

Note: This proposal does not cover AssumptionFailedError or InvalidArgumentException(LogicException) etc

Proposed solution

  • Either convert all \RuntimeException usages to more appropriate exception types, or prohibit the direct use of \RuntimeException within the PM source code.
  • Additionally, a custom PHPStan rule has been introduced to issue warnings when \RuntimeException is thrown directly.
  • The exceptions.check.missingCheckedExceptionInThrows phpstan rule has also been enabled to further strengthen the detection of exception leaks, particularly in relation to supply chain attack vectors.

For example, \RuntimeException could be replaced with an exception that represents an pocketmine\utils\ExpectedCrashException.

The PHPStan custom rule used to prohibit \RuntimeException is as follows(Written by AI):

<?php

declare(strict_types=1);

namespace tests\phpstan\rule;

use PhpParser\Node\Expr\Throw_;
use PhpParser\Node\Expr\New_;
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrorBuilder;
use PHPStan\Reflection\ReflectionProvider;

/**
 * @implements Rule<Throw_>
 */
final class NoBareRuntimeExceptionRule implements Rule
{
	public function __construct(
		private ReflectionProvider $reflectionProvider
	) {}

	public function getNodeType(): string
	{
		return Throw_::class;
	}

	public function processNode(\PhpParser\Node $node, Scope $scope): array
	{
		$expr = $node->expr;

		if ($expr instanceof New_ && $expr->class instanceof \PhpParser\Node\Name) {
			$className = (string) $expr->class;

			if ($className === \RuntimeException::class) {
				return [
					RuleErrorBuilder::message('Throwing RuntimeException directly is not allowed. Use a subclass instead.')
						->line($node->getLine())
						->identifier('no.bare.runtime')
						->build()
				];
			}

			if ($this->reflectionProvider->hasClass($className)) {
				$classReflection = $this->reflectionProvider->getClass($className);
				if ($classReflection->isSubclassOf(\RuntimeException::class) && $classReflection->getName() === \RuntimeException::class) {
					return [
						RuleErrorBuilder::message('Throwing RuntimeException directly is not allowed. Use a subclass instead.')
							->line($node->getLine())
							->identifier('no.bare.runtime')
							->build()
					];
				}
			}
		}

		return [];
	}
}

Alternative solutions or workarounds

No response

Metadata

Metadata

Assignees

No one assigned

    Labels

    Category: CoreRelated to internal functionalityOpinions WantedRequest for comments & opinions from the community

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions