Update abspath sniff#3072
Conversation
📝 WalkthroughWalkthroughA PHP CodeSniffer sniff was refactored to detect direct file access in both HTML-starting and PHP-starting files. The logic now registers for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| PHP |  |
Apr 10, 2026 6:55p.m. | Review ↗ |
| JavaScript | |
Apr 10, 2026 6:55p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php`:
- Around line 178-187: The code computes $insertAfter via findInsertPosition()
but then overwrites the PHP open tag by calling
$phpcsFile->fixer->replaceToken($stackPtr, $content); instead, insert the guard
at the computed position so file-level docblocks stay above it: use the fixer
insertion API (e.g. $phpcsFile->fixer->addContentBefore($insertAfter + 1,
$content) or addContentAfter($insertAfter, $content)) inside
addAbspathCheckToPhpFile() instead of replaceToken($stackPtr, ...), keeping the
beginChangeset()/endChangeset() usage and the same $content variable.
- Around line 58-60: The current logic returns when $stackPtr !== 0 and later
treats a T_INLINE_HTML at position 0 as HTML; instead detect if $tokens[0] is
T_INLINE_HTML and contains only whitespace/BOM (e.g., after trimming
"\xEF\xBB\xBF" and whitespace) then skip it by finding the first T_OPEN_TAG and
set $stackPtr to that index so the file is processed as PHP; update the code
path that routes T_INLINE_HTML to the HTML fixer to only do so for
non-whitespace content, and use the existing token array ($tokens) and token
constants (T_INLINE_HTML, T_OPEN_TAG) to locate and switch the starting token
before invoking the HTML/PHP fixer logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dd1139d9-27db-40d2-ac1d-e457bb255fdd
📒 Files selected for processing (1)
phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php
| // Only process the first token in the file (whether T_OPEN_TAG or T_INLINE_HTML). | ||
| if ( $stackPtr !== 0 ) { | ||
| return; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, get the full file to examine the logic around lines 135-165
wc -l phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.phpRepository: Strategy11/formidable-forms
Length of output: 148
🏁 Script executed:
# Read the relevant sections: around line 136 and the addAbspathCheck method
cat -n phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php | sed -n '120,170p'Repository: Strategy11/formidable-forms
Length of output: 2007
🏁 Script executed:
# Look for the addAbspathCheckToHtmlFile method to see how it generates output
cat -n phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php | sed -n '170,220p'Repository: Strategy11/formidable-forms
Length of output: 1974
🏁 Script executed:
# Look for the addAbspathCheckToPhpFile method to understand the PHP branch
cat -n phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php | sed -n '220,268p'Repository: Strategy11/formidable-forms
Length of output: 1824
🌐 Web query:
PHP_CodeSniffer tokenization BOM T_INLINE_HTML first token
💡 Result:
In PHP_CodeSniffer (PHPCS), when a PHP file starts with a UTF-8 Byte Order Mark (BOM, bytes EF BB BF), the tokenizer includes the BOM as part of the content of the first T_INLINE_HTML token. The first token in the file is T_INLINE_HTML, and its 'content' property begins with the BOM characters followed by any subsequent HTML or whitespace before the first PHP opening tag (?php). This behavior occurs because PHP's token_get_all (used by PHPCS tokenizer) treats initial non-PHP content, including BOM, as T_INLINE_HTML. The BOM is not tokenized separately but embedded in this first token's content. The Generic.Files.ByteOrderMark sniff detects this by registering for T_INLINE_HTML tokens and checking if the first such token's content (trimmed) starts with the UTF-8 BOM signature "\xEF\xBB\xBF". If found at position 0, it reports a violation. The sniff optimizes by checking only once and skipping the rest of the file by returning $phpcsFile->numTokens. This is confirmed by recent PHPCS source analysis (2024 PRs #360, #278) where the sniff explicitly processes T_INLINE_HTML for BOM detection at file start, and Tokenizer.php code handling T_INLINE_HTML position mapping.
Citations:
- 1: Generic/ByteOrderMark: small performance improvement PHPCSStandards/PHP_CodeSniffer#360
- 2: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Tokenizers/Tokenizer.php
- 3: https://pear.php.net/reference/PHP_CodeSniffer-1.5.2/PHP_CodeSniffer/Generic_Sniffs_Files_ByteOrderMarkSniff.html
- 4: https://apidoc.gitee.com/leqq00/tp5/classes/PHP_CodeSniffer.Standards.Generic.Sniffs.Files.ByteOrderMarkSniff.html
- 5: Generic/ByteOrderMark: improve code coverage PHPCSStandards/PHP_CodeSniffer#278
- 6: https://pear.php.net/reference/PHP_CodeSniffer-3.1.1/PHP_CodeSniffer/_PHP_CodeSniffer-3.1.1---src---Standards---Generic---Sniffs---Files---ByteOrderMarkSniff.php.html
🌐 Web query:
PHP_CodeSniffer leading whitespace tokenization file start
💡 Result:
In PHP_CodeSniffer (PHPCS), leading whitespace at the start of a PHP file before the opening PHP tag (<?php) is tokenized as a separate T_INLINE_HTML token. This occurs because PHPCS uses PHP's built-in token_get_all function in its PHPCSTokenizer (the concrete implementation extending the base Tokenizer class), which tokenizes content before the first PHP open tag as T_INLINE_HTML. The T_OPEN_TAG token itself includes any whitespace immediately following the '<?php' (e.g., a newline or space), as this is how PHP's tokenizer behaves—a PHP open tag must be followed by whitespace, and that trailing whitespace is part of the T_OPEN_TAG token content. PHPCS does not split this by default but preserves PHP's tokenization behavior. Sniffs like Squiz.WhiteSpace.SuperfluousWhitespace detect "Additional whitespace found at start of file" by checking for T_INLINE_HTML tokens at position 0 with non-empty content. Generic.PHP.CharacterBeforePHPOpenTag also flags non-whitespace characters before the open tag. Recent proposals (PHPCSStandards/PHP_CodeSniffer issue #593) discuss improving consistency by potentially splitting trailing whitespace from T_OPEN_TAG into a separate T_WHITESPACE token, but as of the latest sources (squizlabs/PHP_CodeSniffer master), it follows PHP's native tokenization. To verify tokenization yourself, use PHPCS with -vvv verbosity on a file starting with whitespace: File content: <?php Tokens (excerpt): Process token 0 on line 1 [lvl:0;]: T_INLINE_HTML => \n Sources confirm PHPCS Tokenizer calls token_get_all on the raw file content, producing T_INLINE_HTML for leading whitespace, followed by T_OPEN_TAG including its trailing whitespace.
Citations:
- 1: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Tokenizers/Tokenizer.php
- 2: Whitespace before PHP open tag sniff ? squizlabs/PHP_CodeSniffer#2038
- 3: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Files/File.php
- 4: 4.0 | Proposal: improve consistency for PHP open tag tokenization PHPCSStandards/PHP_CodeSniffer#593
- 5: T_OPEN_TAG with white character squizlabs/PHP_CodeSniffer#1978
- 6: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/src/Standards/Squiz/Sniffs/PHP/EmbeddedPhpSniff.php
- 7: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage
- 8: Superfluous whitespace squizlabs/PHP_CodeSniffer#317
- 9: https://stackoverflow.com/questions/29901008/codesniffer-to-check-for-whitespace-lines
Skip whitespace-only leading tokens when routing to the HTML fixer.
Lines 136–138 route any first T_INLINE_HTML token to the HTML fixer. However, PHP_CodeSniffer tokenizes BOM (UTF-8 byte order mark) and leading whitespace before <?php as T_INLINE_HTML. When the HTML fixer replaces this token with <?php\n{check}\n\n?>\n{content}, it places the original whitespace/BOM after the ?> closing tag, converting invisible trivia into visible output and corrupting the file.
Check if the T_INLINE_HTML token at position 0 contains only whitespace. If so, skip it and find the actual T_OPEN_TAG to process as a PHP file instead.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php`
around lines 58 - 60, The current logic returns when $stackPtr !== 0 and later
treats a T_INLINE_HTML at position 0 as HTML; instead detect if $tokens[0] is
T_INLINE_HTML and contains only whitespace/BOM (e.g., after trimming
"\xEF\xBB\xBF" and whitespace) then skip it by finding the first T_OPEN_TAG and
set $stackPtr to that index so the file is processed as PHP; update the code
path that routes T_INLINE_HTML to the HTML fixer to only do so for
non-whitespace content, and use the existing token array ($tokens) and token
constants (T_INLINE_HTML, T_OPEN_TAG) to locate and switch the starting token
before invoking the HTML/PHP fixer logic.
| private function addAbspathCheckToPhpFile( File $phpcsFile, $stackPtr ) { | ||
| $tokens = $phpcsFile->getTokens(); | ||
|
|
||
| // Find where to insert the check. | ||
| $insertAfter = $this->findInsertPosition( $phpcsFile, $stackPtr ); | ||
|
|
||
| $phpcsFile->fixer->beginChangeset(); | ||
|
|
||
| // Replace the <?php tag with <?php + newline + ABSPATH check + blank line. | ||
| $phpcsFile->fixer->replaceToken( $stackPtr, "<?php\n" . $this->accessCheck . "\n\n" ); | ||
| $content = '<?php' . chr( 10 ) . $this->accessCheck . chr( 10 ) . chr( 10 ); | ||
| $phpcsFile->fixer->replaceToken( $stackPtr, $content ); |
There was a problem hiding this comment.
Use the computed insertion point for PHP-starting files.
Line 182 still computes $insertAfter, but Line 187 always rewrites the opening tag instead of inserting at that offset. That moves the guard above any file-level docblock, which is the opposite of what findInsertPosition() is supposed to preserve.
💡 Suggested fix
private function addAbspathCheckToPhpFile( File $phpcsFile, $stackPtr ) {
- $tokens = $phpcsFile->getTokens();
-
// Find where to insert the check.
$insertAfter = $this->findInsertPosition( $phpcsFile, $stackPtr );
$phpcsFile->fixer->beginChangeset();
- $content = '<?php' . chr( 10 ) . $this->accessCheck . chr( 10 ) . chr( 10 );
- $phpcsFile->fixer->replaceToken( $stackPtr, $content );
-
- $nextNonWhitespace = $phpcsFile->findNext( T_WHITESPACE, $stackPtr + 1, null, true );
-
- if ( false !== $nextNonWhitespace ) {
- for ( $i = $stackPtr + 1; $i < $nextNonWhitespace; $i++ ) {
- $phpcsFile->fixer->replaceToken( $i, '' );
- }
- }
+ if ( $insertAfter === $stackPtr ) {
+ $content = '<?php' . chr( 10 ) . $this->accessCheck . chr( 10 ) . chr( 10 );
+ $phpcsFile->fixer->replaceToken( $stackPtr, $content );
+
+ $nextNonWhitespace = $phpcsFile->findNext( T_WHITESPACE, $stackPtr + 1, null, true );
+ if ( false !== $nextNonWhitespace ) {
+ for ( $i = $stackPtr + 1; $i < $nextNonWhitespace; $i++ ) {
+ $phpcsFile->fixer->replaceToken( $i, '' );
+ }
+ }
+ } else {
+ $phpcsFile->fixer->addContentAfter(
+ $insertAfter,
+ chr( 10 ) . $this->accessCheck . chr( 10 ) . chr( 10 )
+ );
+ }
$phpcsFile->fixer->endChangeset();
}🧰 Tools
🪛 PHPMD (2.15.0)
[warning] 179-179: Avoid unused local variables such as '$tokens'. (undefined)
(UnusedLocalVariable)
[warning] 182-182: Avoid unused local variables such as '$insertAfter'. (undefined)
(UnusedLocalVariable)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@phpcs-sniffs/Formidable/Sniffs/Security/AddDirectFileAccessCheckSniff.php`
around lines 178 - 187, The code computes $insertAfter via findInsertPosition()
but then overwrites the PHP open tag by calling
$phpcsFile->fixer->replaceToken($stackPtr, $content); instead, insert the guard
at the computed position so file-level docblocks stay above it: use the fixer
insertion API (e.g. $phpcsFile->fixer->addContentBefore($insertAfter + 1,
$content) or addContentAfter($insertAfter, $content)) inside
addAbspathCheckToPhpFile() instead of replaceToken($stackPtr, ...), keeping the
beginChangeset()/endChangeset() usage and the same $content variable.
1 participant
It was adding the <?php tags inside of HTML, not at the top of the file.
Summary by CodeRabbit