Skip to content

Swap out direct file access test for phpcs sniff#2876

Merged
Crabcyborg merged 2 commits into
masterfrom
swap_out_direct_file_access_text_for_phpcs_sniff
Jan 23, 2026
Merged

Swap out direct file access test for phpcs sniff#2876
Crabcyborg merged 2 commits into
masterfrom
swap_out_direct_file_access_text_for_phpcs_sniff

Conversation

@Crabcyborg

@Crabcyborg Crabcyborg commented Jan 23, 2026

Copy link
Copy Markdown
Contributor

This should help in a few ways

  • It's easier to apply in multiple plugins without needing to add the test in multiple repos.
  • It's capable of auto-fixing the issue, unlike the unit test.
  • The unit tests are slower than our phpcs workflows, so this should help make our unit tests faster.

Summary by CodeRabbit

  • New Features
    • Added automated security validation to ensure proper direct file access protection is implemented throughout the codebase
    • Automatically identifies and applies fixes to code missing required security measures
    • Intelligently determines the optimal position for inserting security measures
    • Enhances security enforcement through integrated automated code quality checks

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 23, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Introduces a new PHP_CodeSniffer sniff to enforce direct file access protection checks (ABSPATH guard) across PHP files. The sniff is registered in the ruleset configuration, and the legacy PHPUnit test that previously validated this pattern manually is removed.

Changes

Cohort / File(s) Summary
New Security Sniff
phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php
Introduces automated token-based detection and fix for missing ABSPATH direct file access guards. Skips test files and rector.php. Implements register(), process(), and helper methods (hasAbspathCheck, addAbspathCheck, findInsertPosition) to scan, report, and inject ABSPATH checks with intelligent insertion positioning after docblocks or PHP open tags.
Ruleset Configuration
phpcs-sniffs/Formidable/ruleset.xml
Registers the new Formidable.Security.AddDirectFileAccessCheck rule in the Security section of the sniff ruleset.
Test Cleanup
tests/phpunit/misc/test_FrmDirectFileAccess.php
Removes legacy PHPUnit test class that manually validated ABSPATH checks via filesystem scanning and assertions. Eliminates all_php_file_paths(), check_for_abspath_check(), and test_direct_file_access() methods.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A sniff so sharp, it checks with care,
For ABSPATH guards everywhere!
No more tests to run by hand,
Automation takes a stand. ✨
Security, automated and grand!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main change: replacing a direct file access unit test with a phpcs sniff, which aligns directly with the changeset modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php`:
- Around line 139-160: The addAbspathCheck method calculates $insertAfter via
findInsertPosition but then ignores it — replace the opening-tag replacement and
the whitespace-cleanup to operate at $insertAfter (not $stackPtr) so the ABSPATH
check is inserted after any file-level docblock; also remove the unused $tokens
= $phpcsFile->getTokens(); declaration. Update the replaceToken call to use
$insertAfter and start the whitespace loop from $insertAfter + 1, keeping the
existing use of $nextNonWhitespace and fixer changeset calls.
🧹 Nitpick comments (1)
phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php (1)

49-51: Remove unused variable $tokens.

The $tokens variable is assigned but never used in the process() method.

♻️ Suggested fix
 public function process( File $phpcsFile, $stackPtr ) {
-	$tokens   = $phpcsFile->getTokens();
 	$filename = $phpcsFile->getFilename();

@Crabcyborg
Crabcyborg merged commit 1f55700 into master Jan 23, 2026
16 checks passed
@Crabcyborg
Crabcyborg deleted the swap_out_direct_file_access_text_for_phpcs_sniff branch January 23, 2026 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant