Skip to content

Harden include-loading rules against masked failures - #1034

Merged
mglaman merged 1 commit into
mainfrom
audit/1b-include-hardening
Aug 5, 2026
Merged

Harden include-loading rules against masked failures#1034
mglaman merged 1 commit into
mainfrom
audit/1b-include-hardening

Conversation

@mglaman

@mglaman mglaman commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Part 3 of 9 in the legacy-code audit stack (on top of #1026).

What changed

LoadIncludes and ModuleLoadInclude wrapped their entire rule body in catch (Throwable), so PHPStan's own internal exceptions were swallowed and reported to users as "file could not be loaded". The catch now wraps only the require_once itself, and the intentional load-the-include side effect is documented in place.

ModuleLoadInclude was also missing the non-constant-args guard that LoadIncludes has, so a non-literal module name interpolated false into its error message. The shared parseLoadIncludeArgs() sentinel return shape is now documented on the base class — the missing annotation is what let the two rules diverge unnoticed.

Testing

Existing LoadIncludes/ModuleLoadInclude rule tests cover the reported messages; full suite, self-analysis, and phpcs are green.

🤖 Generated with Claude Code

@mglaman
mglaman force-pushed the audit/1b-include-hardening branch from 5bc7d7d to a178ab7 Compare August 5, 2026 17:15
Base automatically changed from audit/1-correctness-fixes to main August 5, 2026 19:13
Narrow the blanket catch (Throwable) in LoadIncludes and
ModuleLoadInclude to wrap only the require_once, so PHPStan's own
exceptions propagate instead of being reported as unloadable files.
Add the missing non-constant-args guard to ModuleLoadInclude, which
previously interpolated false into its error message, and document
parseLoadIncludeArgs()'s sentinel return shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@mglaman
mglaman force-pushed the audit/1b-include-hardening branch from a178ab7 to 20e6baf Compare August 5, 2026 19:13
@mglaman
mglaman marked this pull request as ready for review August 5, 2026 19:14
@mglaman
mglaman merged commit b61bc8c into main Aug 5, 2026
19 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant