diff --git a/.github/workflows/code-rules.yml b/.github/workflows/code-rules.yml new file mode 100644 index 000000000000..a0c69de751ec --- /dev/null +++ b/.github/workflows/code-rules.yml @@ -0,0 +1,63 @@ +name: code-rules +on: [pull_request, push] +permissions: + contents: read + pull-requests: read +jobs: + code-rules: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + with: + fetch-depth: 1 + + - name: Setup PHP + uses: shivammathur/setup-php@v2 + with: + php-version: '8.4' + extensions: dom, curl, libxml, mbstring, zip, gd, json, readline, xsl, imagick + tools: composer:v2 + coverage: none + + - name: Cache Composer dependencies + uses: actions/cache@v4 + with: + path: vendor/composer/vendor + key: composer-${{ hashFiles('composer.lock') }} + restore-keys: composer- + + - name: Prepare Customizing directory + # The composer classmap includes ./public/Customizing/global/plugins; --no-scripts + # skips the pre-install-cmd that would create it, so create it here — otherwise + # autoload generation aborts ("Could not scan for classes inside ..."). + run: mkdir -p public/Customizing/global/plugins + + - name: Install Composer packages + # --no-scripts skips the post-autoload-dump build (cli/setup.php build), which + # copies front-end assets from node_modules. PHPStan only needs the autoloader + # and classmap (still generated), so neither the build nor Node.js is required. + run: composer install --no-interaction --no-progress --no-scripts + + - name: Cache PHPStan result cache + uses: actions/cache@v4 + with: + path: tmp/phpstan-code-rules + key: phpstan-code-rules-${{ github.run_id }} + restore-keys: phpstan-code-rules- + + - name: ILIAS Code Rules + # Non-blocking for now: the gate reports violations (annotations + report) but + # does not fail the job while the existing findings are being fixed / exempted. + # Remove continue-on-error to make it a hard gate once the count reaches zero. + continue-on-error: true + run: scripts/PHPStan/run_code_rules.sh + env: + GHRUN: "yes" + ERROR_FORMAT: github + + - name: Code Rules Summary + if: always() + run: ERROR_FORMAT=stepSummary scripts/PHPStan/run_code_rules.sh >> "$GITHUB_STEP_SUMMARY" || true + env: + GHRUN: "yes" diff --git a/.gitignore b/.gitignore index 733cee9c9d6d..ac74b1c7a1a7 100755 --- a/.gitignore +++ b/.gitignore @@ -9,6 +9,7 @@ /.settings /errors /extern +/tmp /components/ILIAS/PHPUnit/config/cfg.phpunit.php /nbproject /templates/default/delos.css.map diff --git a/components/ILIAS/Init/classes/ErrorHandling/class.ilDelegatingHandler.php b/components/ILIAS/Init/classes/ErrorHandling/class.ilDelegatingHandler.php index 221a908a9030..a4de4a9f510b 100755 --- a/components/ILIAS/Init/classes/ErrorHandling/class.ilDelegatingHandler.php +++ b/components/ILIAS/Init/classes/ErrorHandling/class.ilDelegatingHandler.php @@ -20,6 +20,7 @@ use Whoops\Handler\Handler; use Whoops\Handler\HandlerInterface; +use ILIAS\Scripts\PHPStan\Attributes\AllowSuperglobalWrite; /** * A Whoops error handler that delegates calls on it self to another handler that is created only in the @@ -32,6 +33,7 @@ * in ilErrorHandling, so this class acts rather dump and asks ilErrorHandling for a handler. * @author Richard Klees */ +#[AllowSuperglobalWrite('The error handler needs to write to SuperGlobals to remove secret data.')] final class ilDelegatingHandler extends Handler { private ?HandlerInterface $current_handler = null; diff --git a/components/ILIAS/Init/classes/class.ilInitialisation.php b/components/ILIAS/Init/classes/class.ilInitialisation.php index 33810d466bb9..ee6aa37c4f02 100755 --- a/components/ILIAS/Init/classes/class.ilInitialisation.php +++ b/components/ILIAS/Init/classes/class.ilInitialisation.php @@ -36,6 +36,7 @@ use ILIAS\ILIASObject\Properties\AdditionalProperties\Icon\Factory as CustomIconFactory; use ILIAS\User\PublicInterface as UserPublicInterface; use ILIAS\Mail\Service\MailService; +use ILIAS\Scripts\PHPStan\Attributes\AllowSuperglobalWrite; // needed for slow queries, etc. if (!isset($GLOBALS['ilGlobalStartTime']) || !$GLOBALS['ilGlobalStartTime']) { @@ -51,6 +52,7 @@ * @version $Id$ * @ingroup ServicesInit */ +#[AllowSuperglobalWrite('Remove unsafe Characters, several other legacy mechanisms...')] class ilInitialisation { /** diff --git a/components/ILIAS/UI/tests/Examples/ExamplesTest.php b/components/ILIAS/UI/tests/Examples/ExamplesTest.php index e3fd422593e5..3ca0fa3a1e62 100755 --- a/components/ILIAS/UI/tests/Examples/ExamplesTest.php +++ b/components/ILIAS/UI/tests/Examples/ExamplesTest.php @@ -25,6 +25,7 @@ use ILIAS\DI\Container; use ILIAS\UI\NotImplementedException; use ILIAS\FileUpload\FileUpload; +use ILIAS\Scripts\PHPStan\Attributes\AllowSuperglobalWrite; /** * Class ExamplesTest Checks if all examples are implemented and properly returning strings @@ -47,6 +48,7 @@ class ExamplesTest extends ILIAS_UI_TestBase protected Container $dic; protected Crawler\ExamplesYamlParser $example_parser; + #[AllowSuperglobalWrite('Bypass Undefined index: ilfilehash for the moment. This is for examples only.')] public function setUp(): void { //This avoids various index not set warnings, which are only relevant in test context. diff --git a/components/ILIAS/WebDAV/src/Request/RequestTranslation.php b/components/ILIAS/WebDAV/src/Request/RequestTranslation.php index bda03fd4bea9..883c129b5f76 100644 --- a/components/ILIAS/WebDAV/src/Request/RequestTranslation.php +++ b/components/ILIAS/WebDAV/src/Request/RequestTranslation.php @@ -23,6 +23,7 @@ use Psr\Http\Message\ServerRequestInterface; use ILIAS\WebDAV\Config; use ILIAS\HTTP\Wrapper\SuperGlobalDropInReplacement; +use ILIAS\Scripts\PHPStan\Attributes\AllowSuperglobalWrite; /** * @internal @@ -92,6 +93,11 @@ public function showMountPoint(): bool return array_key_exists($this->config->getMountInstructionsQuery(), $this->request->getQueryParams()); } + #[AllowSuperglobalWrite( + 'as long as we use ' + . \ILIAS\HTTP\Wrapper\SuperGlobalDropInReplacement::class + . ', we must cast back to array to make SabreDAV work.' + )] public function setup(): void { $this->post = $_POST; diff --git a/scripts/PHPStan/Attributes/AllowRuleViolation.php b/scripts/PHPStan/Attributes/AllowRuleViolation.php new file mode 100644 index 000000000000..509f20d0f7c6 --- /dev/null +++ b/scripts/PHPStan/Attributes/AllowRuleViolation.php @@ -0,0 +1,63 @@ +` in the analysis output, e.g. `ilias.superglobalWrite`). Pass one + * or more; an allowance with no identifiers exempts nothing. + * + * PHP attributes can only sit on declarations, so this exempts the whole + * class/method/function. For a single free-standing statement (e.g. in a resource + * script without an enclosing function) use an inline + * `// @phpstan-ignore (reason)` comment instead. + * + * For frequently exempted rules there are convenience subclasses that already carry + * the identifier, e.g. {@see AllowSuperglobalWrite}. The checker matches those via + * `ReflectionAttribute::IS_INSTANCEOF`, so subclasses work exactly like the base + * attribute. + * + * Example: + * ```php + * #[AllowRuleViolation('sanitizes $_GET before the HTTP service exists', 'ilias.superglobalWrite')] + * public static function recursivelyRemoveUnsafeCharacters(): void { ... } + * ``` + */ +#[Attribute(Attribute::TARGET_CLASS | Attribute::TARGET_METHOD | Attribute::TARGET_FUNCTION)] +readonly class AllowRuleViolation +{ + /** @var list */ + public array $rules; + + public function __construct(public string $reason, string ...$rules) + { + $this->rules = array_values($rules); + } +} diff --git a/scripts/PHPStan/Attributes/AllowSuperglobalWrite.php b/scripts/PHPStan/Attributes/AllowSuperglobalWrite.php new file mode 100644 index 000000000000..9dc9f838c004 --- /dev/null +++ b/scripts/PHPStan/Attributes/AllowSuperglobalWrite.php @@ -0,0 +1,42 @@ +getAttributes( + AllowRuleViolation::class, + \ReflectionAttribute::IS_INSTANCEOF + ) as $attribute) { + $allowance = $attribute->newInstance(); + if (in_array($rule_identifier, $allowance->rules, true)) { + return true; + } + } + } + } catch (\Throwable) { + // Reflection can fail for not-yet-loadable symbols; never break analysis over it. + return false; + } + + return false; + } + + /** + * @return iterable<\ReflectionClass|\ReflectionMethod|\ReflectionFunction> + */ + private static function reflections(Scope $scope): iterable + { + $class_reflection = $scope->getClassReflection(); + if ($class_reflection !== null) { + $native_class = $class_reflection->getNativeReflection(); + yield $native_class; + + $function_name = $scope->getFunctionName(); + if ($function_name !== null && $native_class->hasMethod($function_name)) { + yield $native_class->getMethod($function_name); + } + + return; + } + + $function = $scope->getFunction(); + if ($function instanceof PhpFunctionReflection) { + yield $function->getNativeReflection(); + } + } +} diff --git a/scripts/PHPStan/ErrorFormatter/StepSummaryFormatter.php b/scripts/PHPStan/ErrorFormatter/StepSummaryFormatter.php new file mode 100644 index 000000000000..72079caa380e --- /dev/null +++ b/scripts/PHPStan/ErrorFormatter/StepSummaryFormatter.php @@ -0,0 +1,99 @@ + $per_rule */ + $per_rule = []; + /** @var array $per_component */ + $per_component = []; + /** @var array> $component_rules */ + $component_rules = []; + + foreach ($analysisResult->getFileSpecificErrors() as $error) { + $total++; + + $identifier = $error->getIdentifier() ?? 'unknown'; + $label = $error->getMetadata()['rule'] ?? $identifier; + $per_rule[$identifier] ??= ['label' => $label, 'count' => 0]; + $per_rule[$identifier]['count']++; + + $component = preg_match(self::COMPONENT_REGEX, $error->getFile(), $matches) + ? $matches[1] + : self::OTHER; + $per_component[$component] = ($per_component[$component] ?? 0) + 1; + $component_rules[$component][$identifier] ??= ['label' => $label, 'count' => 0]; + $component_rules[$component][$identifier]['count']++; + } + + uasort($per_rule, static fn(array $a, array $b): int => $b['count'] <=> $a['count']); + arsort($per_component); + + $output->writeLineFormatted('## ILIAS Code Rules — ' . $total . ' violation' . ($total === 1 ? '' : 's')); + $output->writeLineFormatted(''); + + if ($total === 0) { + $output->writeLineFormatted('No violations. :white_check_mark:'); + return 0; + } + + $output->writeLineFormatted('| Rule | Violations |'); + $output->writeLineFormatted('|---|--:|'); + foreach ($per_rule as $identifier => $info) { + $output->writeLineFormatted('| ' . $info['label'] . ' (`' . $identifier . '`) | ' . $info['count'] . ' |'); + } + + $output->writeLineFormatted(''); + $output->writeLineFormatted('**Top components**'); + $output->writeLineFormatted(''); + foreach (array_slice($per_component, 0, self::TOP_COMPONENTS, true) as $component => $count) { + $output->writeLineFormatted('- **' . $component . '** — ' . $count); + + $rules = $component_rules[$component]; + uasort($rules, static fn(array $a, array $b): int => $b['count'] <=> $a['count']); + foreach ($rules as $info) { + $output->writeLineFormatted(' - ' . $info['label'] . ': ' . $info['count']); + } + } + + return 1; + } +} diff --git a/scripts/PHPStan/README.md b/scripts/PHPStan/README.md index e024d690d9da..2c36dfde9158 100755 --- a/scripts/PHPStan/README.md +++ b/scripts/PHPStan/README.md @@ -1,6 +1,114 @@ PHPStan Custom Rules ==================== +This directory holds ILIAS' custom PHPStan rules. They come in two independent +rulesets with different purposes and lifecycles: + +| Ruleset | Config | Purpose | CI | +|---|---|---|---| +| Legacy-UI report | `legacy_ui.neon` | Track migration of legacy UI components | `legacy-ui.yml` — nightly, non-gating, uploads a CSV report | +| Code rules | `code_rules.neon` | Enforce code policies | `code-rules.yml` — per pull request, **hard gate** | + +The rule classes for both live under `Rules/`, grouped into one subdirectory (and +matching namespace) per topic, and are autoloaded via composer PSR-4 +(`ILIAS\Scripts` → `./scripts`): + +``` +Rules/ + SuperGlobals/ ILIAS\Scripts\PHPStan\Rules\SuperGlobals — no-superglobal-write rule + LegacyUI/ ILIAS\Scripts\PHPStan\Rules\LegacyUI — legacy UI component rules +Attributes/ ILIAS\Scripts\PHPStan\Attributes — the AllowRuleViolation exemption attribute + checker +``` + +Add a new topic as a new subdirectory/namespace under `Rules/`. + +Code Rules (policy gate) +------------------------ + +The code-rules ruleset enforces mandatory coding policies. It is a hard gate: the +CI job `code-rules.yml` fails a pull request on any violation. The policies are +enforced everywhere; genuinely unavoidable cases are exempted explicitly in the code +(see [Exempting a violation](#exempting-a-violation) below), not silently +grandfathered. + +Run the full gate locally: + +```bash +./scripts/PHPStan/run_code_rules.sh +``` + +Run it for a single directory (e.g. `components/ILIAS/File`): + +```bash +./scripts/PHPStan/run_code_rules.sh components/ILIAS/File +``` + +CI runs the same script with `ERROR_FORMAT=github` so violations show up as inline +annotations on the pull request; any PHPStan `--error-format` can be set via that +environment variable. `ERROR_FORMAT=stepSummary` renders the Markdown summary that CI +appends to the GitHub step summary. + +### Active policies + +| Rule classes | Identifier | Policy | +|---|---|---| +| `SuperglobalAssignRule`, `SuperglobalAssignOpRule`, `SuperglobalAssignRefRule` | `ilias.superglobalWrite` | No writing to request-input superglobals (`$_GET`, `$_POST`, `$_REQUEST`, `$_COOKIE`, `$_FILES`). The request is immutable — use the HTTP service / request wrapper instead. | + +### Adding a policy rule + +1. Create a subdirectory under `Rules/` for the topic (namespace + `ILIAS\Scripts\PHPStan\Rules\`) and add a rule class implementing + `PHPStan\Rules\Rule` (or extending a shared base). Expose two constants on it: + - `public const IDENTIFIER = 'ilias.';` — the stable error identifier, + passed to `->identifier(self::IDENTIFIER)` and referenced by exemptions. + - `public const LABEL = '';` — the human-readable name. Pass it into + the error metadata (`->metadata(['rule' => self::LABEL, ...])`); the step-summary + error formatter reads the label from there, so the name has a single source. + + For a rule family that shares one identifier across several node-type subclasses + (e.g. the superglobal / `$GLOBALS` rules), put both constants on the abstract base. +2. Register the class under `rules:` in `code_rules.neon`. There is nothing else to + wire up: the gate runs it from that list, and the step-summary formatter + (`ErrorFormatter/StepSummaryFormatter`, registered as the `stepSummary` error + format) picks up its label straight from the error metadata. +3. Run the gate and either fix the code it flags or exempt the unavoidable cases + (next section). The gate must be green before the rule is merged. + +### Exempting a violation + +There is no blanket baseline. Genuinely unavoidable violations are exempted +explicitly and locally, so every exception is visible and carries a reason: + +- **On a class, method or function** — annotate it with the + `ILIAS\Scripts\PHPStan\Attributes\AllowRuleViolation` attribute, passing a reason + and one or more rule identifiers: + + ```php + use ILIAS\Scripts\PHPStan\Attributes\AllowRuleViolation; + + #[AllowRuleViolation('populates $_GET before the HTTP service exists', 'ilias.superglobalWrite')] + public static function sanitizeRequest(): void { /* … */ } + ``` + + Some rule sets additionally ship a convenience subclass of `AllowRuleViolation` + that already carries the identifier, so only the reason is needed. The checker + matches those via `ReflectionAttribute::IS_INSTANCEOF`. + +- **On a single free-standing statement** (e.g. in a resource script without an + enclosing function, where an attribute cannot be placed) — use an inline comment + with the identifier and a reason: + + ```php + $_COOKIE['ilClientId'] = $client_id; // @phpstan-ignore ilias.superglobalWrite (bootstrap before request wrapper) + ``` + +There is no baseline file: the gate must stay green through in-code exemptions, not +by grandfathering. (If a mass migration ever needs one, `--generate-baseline` can +create it and add its `includes:` entry back to `code_rules.neon`.) + +Legacy-UI report +---------------- + With the ["Removing of Legacy-UIComponents-Service and Table" project](https://docu.ilias.de/goto_docu_grp_12110.html), a large number of UI elements that are not available in the UI service will be replaced by ILIAS 10. With the rules collected here, violations of the deprecations are found and collected in reports. The entire report comprises a CSV file for each component and a summarised file for the entire code base, this form of the report can be generated as follows: diff --git a/scripts/PHPStan/Rules/LegacyClassUsageRule.php b/scripts/PHPStan/Rules/LegacyUI/LegacyClassUsageRule.php similarity index 99% rename from scripts/PHPStan/Rules/LegacyClassUsageRule.php rename to scripts/PHPStan/Rules/LegacyUI/LegacyClassUsageRule.php index 0ac9a22dc684..d9f411753edd 100755 --- a/scripts/PHPStan/Rules/LegacyClassUsageRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/LegacyClassUsageRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PhpParser\Node\Expr\CallLike; use PHPStan\Reflection\ReflectionProvider; diff --git a/scripts/PHPStan/Rules/NoLegacyButtonUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyButtonUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyButtonUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyButtonUsagesRule.php index 18a334231a04..f85ce28ed0b8 100755 --- a/scripts/PHPStan/Rules/NoLegacyButtonUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyButtonUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyCheckboxListUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyCheckboxListUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyCheckboxListUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyCheckboxListUsagesRule.php index eb2dc41c48e7..9d335aabc66a 100755 --- a/scripts/PHPStan/Rules/NoLegacyCheckboxListUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyCheckboxListUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyConfirmationUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyConfirmationUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyConfirmationUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyConfirmationUsagesRule.php index cc2b60f5d9c3..6f22983401a9 100755 --- a/scripts/PHPStan/Rules/NoLegacyConfirmationUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyConfirmationUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyExplorerUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyExplorerUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyExplorerUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyExplorerUsagesRule.php index 2bc4d219cc1d..9cbabe41b5ee 100755 --- a/scripts/PHPStan/Rules/NoLegacyExplorerUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyExplorerUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyLightboxUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyLightboxUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyLightboxUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyLightboxUsagesRule.php index 22eb077faf90..19b7dd7f3c4c 100755 --- a/scripts/PHPStan/Rules/NoLegacyLightboxUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyLightboxUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyModalUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyModalUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyModalUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyModalUsagesRule.php index ebdea5a2316f..8cbc705d5867 100755 --- a/scripts/PHPStan/Rules/NoLegacyModalUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyModalUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyNestedListUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyNestedListUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyNestedListUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyNestedListUsagesRule.php index 0d717862afb5..170465fdb39f 100755 --- a/scripts/PHPStan/Rules/NoLegacyNestedListUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyNestedListUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyPanelUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyPanelUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyPanelUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyPanelUsagesRule.php index 5ed1fa6d9df5..b80418749714 100755 --- a/scripts/PHPStan/Rules/NoLegacyPanelUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyPanelUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyProgressBarUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyProgressBarUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyProgressBarUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyProgressBarUsagesRule.php index 4d693e5f19cd..6e2bd93aa060 100755 --- a/scripts/PHPStan/Rules/NoLegacyProgressBarUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyProgressBarUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacySyntaxHighlighterUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacySyntaxHighlighterUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacySyntaxHighlighterUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacySyntaxHighlighterUsagesRule.php index 2101990e7b03..97a2582f056c 100755 --- a/scripts/PHPStan/Rules/NoLegacySyntaxHighlighterUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacySyntaxHighlighterUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyTableUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyTableUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyTableUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyTableUsagesRule.php index 68ed9e52ccdc..cf2459efec45 100755 --- a/scripts/PHPStan/Rules/NoLegacyTableUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyTableUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyTabsUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyTabsUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyTabsUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyTabsUsagesRule.php index 8b0fdef7a7d6..47dba2c15511 100755 --- a/scripts/PHPStan/Rules/NoLegacyTabsUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyTabsUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyTextHighlighterUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyTextHighlighterUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyTextHighlighterUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyTextHighlighterUsagesRule.php index 4ebdfbf2a035..228416a90088 100755 --- a/scripts/PHPStan/Rules/NoLegacyTextHighlighterUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyTextHighlighterUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/NoLegacyToolbarUsagesRule.php b/scripts/PHPStan/Rules/LegacyUI/NoLegacyToolbarUsagesRule.php similarity index 95% rename from scripts/PHPStan/Rules/NoLegacyToolbarUsagesRule.php rename to scripts/PHPStan/Rules/LegacyUI/NoLegacyToolbarUsagesRule.php index 634384d34e0b..ab0d4082842b 100755 --- a/scripts/PHPStan/Rules/NoLegacyToolbarUsagesRule.php +++ b/scripts/PHPStan/Rules/LegacyUI/NoLegacyToolbarUsagesRule.php @@ -16,7 +16,7 @@ * *********************************************************************/ -namespace ILIAS\Scripts\PHPStan\Rules; +namespace ILIAS\Scripts\PHPStan\Rules\LegacyUI; use PHPStan\Reflection\ReflectionProvider; use PHPStan\Rules\Rule; diff --git a/scripts/PHPStan/Rules/SuperGlobals/AbstractSuperglobalWriteRule.php b/scripts/PHPStan/Rules/SuperGlobals/AbstractSuperglobalWriteRule.php new file mode 100644 index 000000000000..e85b6210d6ce --- /dev/null +++ b/scripts/PHPStan/Rules/SuperGlobals/AbstractSuperglobalWriteRule.php @@ -0,0 +1,120 @@ + + */ + protected function getForbiddenSuperglobals(): array + { + return ['_GET', '_POST', '_REQUEST', '_COOKIE', '_FILES']; + } + + final public function processNode(Node $node, Scope $scope): array + { + // All handled assignment nodes expose the write target as `$var`. + if (!isset($node->var) || !$node->var instanceof Expr) { + return []; + } + + $superglobal = $this->findWrittenSuperglobal($node->var); + if ($superglobal === null) { + return []; + } + + if (RuleViolationAllowance::isAllowedIn($scope, self::IDENTIFIER)) { + return []; + } + + return [ + RuleErrorBuilder::message( + "Writing to the superglobal \$$superglobal is forbidden. " + . 'The request is immutable; use the HTTP service / request wrapper instead.' + ) + ->identifier(self::IDENTIFIER) + ->metadata([ + 'rule' => self::LABEL, + 'version' => 12, + ]) + ->build() + ]; + } + + /** + * Walks an assignment target down to its root variable and returns the + * superglobal name (without `$`) if that root is a forbidden superglobal. + * + * Covers `$_GET['x'] = …`, nested dimensions `$_GET['a']['b'] = …`, + * appends `$_GET[] = …` and whole-array overwrites `$_GET = …`. + */ + private function findWrittenSuperglobal(Expr $target): ?string + { + while ($target instanceof ArrayDimFetch) { + $target = $target->var; + } + + if (!$target instanceof Variable || !is_string($target->name)) { + return null; + } + + return in_array($target->name, $this->getForbiddenSuperglobals(), true) + ? $target->name + : null; + } +} diff --git a/scripts/PHPStan/Rules/SuperGlobals/SuperglobalAssignOpRule.php b/scripts/PHPStan/Rules/SuperGlobals/SuperglobalAssignOpRule.php new file mode 100644 index 000000000000..86687161e93c --- /dev/null +++ b/scripts/PHPStan/Rules/SuperGlobals/SuperglobalAssignOpRule.php @@ -0,0 +1,40 @@ + + */ +final class SuperglobalAssignOpRule extends AbstractSuperglobalWriteRule +{ + public function getNodeType(): string + { + return AssignOp::class; + } +} diff --git a/scripts/PHPStan/Rules/SuperGlobals/SuperglobalAssignRefRule.php b/scripts/PHPStan/Rules/SuperGlobals/SuperglobalAssignRefRule.php new file mode 100644 index 000000000000..e480a499b6dd --- /dev/null +++ b/scripts/PHPStan/Rules/SuperGlobals/SuperglobalAssignRefRule.php @@ -0,0 +1,36 @@ + + */ +final class SuperglobalAssignRefRule extends AbstractSuperglobalWriteRule +{ + public function getNodeType(): string + { + return AssignRef::class; + } +} diff --git a/scripts/PHPStan/Rules/SuperGlobals/SuperglobalAssignRule.php b/scripts/PHPStan/Rules/SuperGlobals/SuperglobalAssignRule.php new file mode 100644 index 000000000000..f7d51b3c9609 --- /dev/null +++ b/scripts/PHPStan/Rules/SuperGlobals/SuperglobalAssignRule.php @@ -0,0 +1,37 @@ + + */ +final class SuperglobalAssignRule extends AbstractSuperglobalWriteRule +{ + public function getNodeType(): string + { + return Assign::class; + } +} diff --git a/scripts/PHPStan/code_rules.neon b/scripts/PHPStan/code_rules.neon new file mode 100644 index 000000000000..4ae8c2be155c --- /dev/null +++ b/scripts/PHPStan/code_rules.neon @@ -0,0 +1,30 @@ +rules: + - ILIAS\Scripts\PHPStan\Rules\SuperGlobals\SuperglobalAssignRule + - ILIAS\Scripts\PHPStan\Rules\SuperGlobals\SuperglobalAssignOpRule + - ILIAS\Scripts\PHPStan\Rules\SuperGlobals\SuperglobalAssignRefRule +services: + errorFormatter.stepSummary: + class: ILIAS\Scripts\PHPStan\ErrorFormatter\StepSummaryFormatter +parameters: + tmpDir: %currentWorkingDirectory%/tmp/phpstan-code-rules + parallel: + maximumNumberOfProcesses: 10 + customRulesetUsed: true + bootstrapFiles: + - constants.php + excludePaths: + - '%currentWorkingDirectory%/vendor/*' + - '%currentWorkingDirectory%/Customizing/*' + - '%currentWorkingDirectory%/scripts/*' + - '%currentWorkingDirectory%/data/*' + - '%currentWorkingDirectory%/dicto/*' + - '%currentWorkingDirectory%/public/*' + - '%currentWorkingDirectory%/docs/*' + - '%currentWorkingDirectory%/lang/*' + - '%currentWorkingDirectory%/soap/lib/*' + - '%currentWorkingDirectory%/node_modules/*' + - '%currentWorkingDirectory%/templates/*' + - '%currentWorkingDirectory%/xml/*' + - '%currentWorkingDirectory%/.github/*' + - '%currentWorkingDirectory%/**/mediawiki/*' + - '%currentWorkingDirectory%/**/Wiki/libs/*' diff --git a/scripts/PHPStan/legacy_ui.neon b/scripts/PHPStan/legacy_ui.neon index 6a0f5aad5dca..3bb872d3eb79 100755 --- a/scripts/PHPStan/legacy_ui.neon +++ b/scripts/PHPStan/legacy_ui.neon @@ -2,20 +2,20 @@ services: errorFormatter.csv: class: \ILIAS\Scripts\PHPStan\ErrorFormatter\CSVFormatter rules: - - ILIAS\Scripts\PHPStan\Rules\NoLegacyButtonUsagesRule # ILIAS 9 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyCheckboxListUsagesRule # ILIAS 9 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyConfirmationUsagesRule # ILIAS 9 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyExplorerUsagesRule # ILIAS 10git - - ILIAS\Scripts\PHPStan\Rules\NoLegacyLightboxUsagesRule # ILIAS 10 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyModalUsagesRule # ILIAS 10 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyNestedListUsagesRule # ILIAS 10 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyPanelUsagesRule # ILIAS 10 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyProgressBarUsagesRule # ILIAS 10 - - ILIAS\Scripts\PHPStan\Rules\NoLegacySyntaxHighlighterUsagesRule # ILIAS 10 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyTableUsagesRule # ILIAS 10 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyTextHighlighterUsagesRule # ILIAS 10 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyToolbarUsagesRule # ILIAS 10 - - ILIAS\Scripts\PHPStan\Rules\NoLegacyTabsUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyButtonUsagesRule # ILIAS 9 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyCheckboxListUsagesRule # ILIAS 9 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyConfirmationUsagesRule # ILIAS 9 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyExplorerUsagesRule # ILIAS 10git + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyLightboxUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyModalUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyNestedListUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyPanelUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyProgressBarUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacySyntaxHighlighterUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyTableUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyTextHighlighterUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyToolbarUsagesRule # ILIAS 10 + - ILIAS\Scripts\PHPStan\Rules\LegacyUI\NoLegacyTabsUsagesRule # ILIAS 10 parameters: tmpDir: %currentWorkingDirectory%/tmp/phpstan parallel: diff --git a/scripts/PHPStan/run_code_rules.sh b/scripts/PHPStan/run_code_rules.sh new file mode 100755 index 000000000000..2e6c00c19080 --- /dev/null +++ b/scripts/PHPStan/run_code_rules.sh @@ -0,0 +1,39 @@ +#!/bin/bash + +# Runs the ILIAS custom code rules (policy gate) via PHPStan. +# +# Hard gate: exits non-zero on any violation, so CI fails the pull request. +# Genuinely unavoidable cases are exempted in the code (AllowRuleViolation +# attribute / inline @phpstan-ignore), not via a baseline. +# +# Environment variables: +# ERROR_FORMAT PHPStan --error-format to use. Default: "table" (human +# readable). CI sets "github" so violations show up as inline +# annotations on the pull request. Any PHPStan format works +# (table, github, json, checkstyle, ...). + +# Never truncate the table output — with a large number of findings the table +# formatter otherwise collapses the list ("... and N more errors"). +export PHPSTAN_TABLE_ERROR_FORMATTER_FORCE_SHOW_ALL_ERRORS=1 + +CONFIG=scripts/PHPStan/code_rules.neon +MEMORY_LIMIT=4G +ERROR_FORMAT="${ERROR_FORMAT:-table}" + +# Target directory: explicit script parameter, or all ILIAS components at once. +if [ -d "$1" ]; then + TARGET="$1" +else + TARGET="components/ILIAS" +fi + +# Informational line goes to stderr so `ERROR_FORMAT=json ... > out.json` stays pure JSON. +echo "Running ILIAS code rules on ${TARGET} (error-format: ${ERROR_FORMAT})" >&2 +php -dxdebug.mode=off vendor/composer/vendor/bin/phpstan analyse \ + -c "${CONFIG}" \ + -a vendor/composer/vendor/autoload.php \ + --no-progress \ + --no-interaction \ + --memory-limit=${MEMORY_LIMIT} \ + --error-format="${ERROR_FORMAT}" \ + "${TARGET}"