Merge pull request #31 from kristianwind/abspath-guard-by-tokens

Check the direct-access guard by tokenising, not by grepping for it

ci/check-abspath.php

@@ -0,0 +1,98 @@
+<?php
+/**
+ * Verify that every shipped PHP file really guards against direct access.
+ *
+ * 🔴 Tokenised, not grepped, and that is the whole point of the file.
+ *
+ * The grep version asked whether "defined( 'ABSPATH' )" appeared ANYWHERE in
+ * the file. A comment saying the guard is handled elsewhere therefore passed,
+ * and so did a file with the words and no guard at all. Measured: a planted
+ * file whose only mention was in a comment produced
+ * "15 shipped PHP files examined, every one guarded" — green over a file that
+ * runs for anyone who fetches it directly, outside WordPress and outside every
+ * capability check the code assumes has already happened.
+ *
+ * A check that must RECOGNISE its own code has one blind spot per recognition
+ * method; one that reads what the source actually says has none of that
+ * particular kind. PHP's own tokeniser cannot be fooled by a comment, because
+ * it is the thing that decides what a comment is.
+ */
+
+$min   = 0;
+$files = array();
+foreach ( array_slice( $argv, 1 ) as $arg ) {
+	if ( 0 === strpos( $arg, '--min=' ) ) {
+		$min = (int) substr( $arg, 6 );
+		continue;
+	}
+	$files[] = $arg;
+}
+
+$guards   = array( 'ABSPATH', 'WP_UNINSTALL_PLUGIN' );
+$problems = 0;
+$checked  = 0;
+
+foreach ( $files as $path ) {
+	if ( ! is_readable( $path ) ) {
+		printf( "::error file=%s::Listed as shipped but not readable.\n", $path );
+		++$problems;
+		continue;
+	}
+
+	++$checked;
+	$tokens = token_get_all( file_get_contents( $path ) );
+	$count  = count( $tokens );
+	$found  = false;
+
+	for ( $i = 0; $i < $count; $i++ ) {
+		$token = $tokens[ $i ];
+
+		// Comments and doc blocks are tokens of their own, so they are simply
+		// never examined. That is the property the grep version lacked.
+		if ( ! is_array( $token ) || T_STRING !== $token[0] || 'defined' !== strtolower( $token[1] ) ) {
+			continue;
+		}
+
+		// defined ( 'ABSPATH' ) — whitespace anywhere, quotes either kind.
+		$j = $i + 1;
+		while ( $j < $count && is_array( $tokens[ $j ] ) && T_WHITESPACE === $tokens[ $j ][0] ) {
+			++$j;
+		}
+		if ( $j >= $count || '(' !== $tokens[ $j ] ) {
+			continue;
+		}
+		++$j;
+		while ( $j < $count && is_array( $tokens[ $j ] ) && T_WHITESPACE === $tokens[ $j ][0] ) {
+			++$j;
+		}
+		if ( $j < $count && is_array( $tokens[ $j ] ) && T_CONSTANT_ENCAPSED_STRING === $tokens[ $j ][0] ) {
+			if ( in_array( trim( $tokens[ $j ][1], "'\"" ), $guards, true ) ) {
+				$found = true;
+				break;
+			}
+		}
+	}
+
+	if ( ! $found ) {
+		printf(
+			"::error file=%s,line=1::No guard against direct access. Add: if ( ! defined( 'ABSPATH' ) ) { exit; }\n",
+			$path
+		);
+		++$problems;
+	}
+}
+
+// The floor, for the same reason every other check in this kit has one: "every
+// shipped file is guarded" is also what zero shipped files prints.
+if ( $checked < $min ) {
+	fwrite( STDERR, sprintf( "Only %d shipped PHP file(s) were examined, expected at least %d. The file list is wrong, so this check proved nothing.\n", $checked, $min ) );
+	exit( 1 );
+}
+
+if ( $problems > 0 ) {
+	printf( "\n%d of %d shipped PHP files have no guard against direct access.\n", $problems, $checked );
+	exit( 1 );
+}
+
+printf( "  %d shipped PHP files parsed, every one guards against direct access\n", $checked );
+exit( 0 );

ci/selftest.sh

@@ -127,10 +127,39 @@ for f in control-hash.txt control-placeholder.txt control-masked.txt \
 done
 
 # The guard check must report a plausible number rather than a bare adjective.
-echo "$out" | grep -qE '^  [0-9]+ shipped PHP files examined' \
+echo "$out" | grep -qE '^  [0-9]+ shipped PHP files parsed' \
   && ok "the direct-access check reports a count" \
   || bad "the direct-access check printed no count — it cannot tell 13 files from 0"
 
+# --------------------------------------------------- direct-access guard
+# 🔴 Grade 2 evidence: a planted instance of exactly the failure the check
+# describes, not an arbitrary trigger. The grep version of this check passed
+# this fixture — the words are present, in a comment, and there is no guard.
+# It reported "every one guarded" over a file that runs for anyone who fetches
+# it directly.
+echo
+echo "→ the direct-access check reads structure, not text"
+cat > includes/class-selftest-unguarded.php <<'FIXTURE'
+<?php
+// This file is safe because defined( 'ABSPATH' ) is checked elsewhere.
+// It is not. There is no guard anywhere below this line.
+echo 'reachable';
+FIXTURE
+git add -N includes/class-selftest-unguarded.php >/dev/null
+# Captured first, NOT piped into grep. This script runs under `set -o
+# pipefail`, and static.sh is expected to exit non-zero here — so
+# `static.sh | grep -q` would report STATIC.SH's status, not grep's, and the
+# assertion would read "not found" however the check behaved. The mirror image
+# of the `go test | grep` trap, and it cost this assertion one red run.
+guard_out=$(./ci/static.sh 2>&1 || true)
+if printf '%s' "$guard_out" | grep -q 'class-selftest-unguarded.php,line=1::No guard'; then
+  ok "a file mentioning the guard only in a comment is still flagged"
+else
+  bad "an unguarded file passed because the words appear in a comment"
+fi
+rm -f includes/class-selftest-unguarded.php
+git rm -q --cached includes/class-selftest-unguarded.php >/dev/null 2>&1 || true
+
 # ---------------------------------------------------------------- text domain
 # 🔴 ci_php, never a bare `php`. The Gitea runner image has NO php — that is
 # why ci_php exists and says so — and this section originally called `php`

ci/static.sh

@@ -50,27 +50,26 @@ done
 rm -f "$tarball"
 
 ci_step "protection against direct access"
-before=$fails
 # Without the guard a PHP file can be requested straight from the browser,
 # outside WordPress and therefore outside every capability check the code
 # assumes has already happened.
-guard_scanned=0
-while read -r f; do
-  [ -n "$f" ] || continue
-  guard_scanned=$((guard_scanned + 1))
-  grep -q "defined( 'ABSPATH' )\|defined( 'WP_UNINSTALL_PLUGIN' )" "$f" \
-    || { ci_annotate error "$f" 1 "Missing the guard against direct access: if ( ! defined( 'ABSPATH' ) ) { exit; }"; fails=$((fails + 1)); }
-done <<< "$(shipped_files)"
-# 🔴 The count is the result, not decoration. "every shipped file is guarded"
-# is also what ZERO shipped files prints, and shipped_files() swallows a
-# dist-files.sh failure with `|| true`, so an empty list is one broken
-# .distignore away. The floor is above 1 on purpose: a pattern that wrongly
-# excludes includes/ leaves the main file behind and collapses the list
-# without emptying it. Proven by stubbing ci/dist-files.sh to print nothing.
-if [ "$guard_scanned" -lt "$MIN_SHIPPED_PHP_FILES" ]; then
-  note_fail "Only $guard_scanned shipped PHP file(s) were examined for the direct-access guard, expected at least $MIN_SHIPPED_PHP_FILES. The file list is wrong, so this check proved nothing."
-elif [ "$fails" = "$before" ]; then
-  echo "  $guard_scanned shipped PHP files examined, every one guarded"
+#
+# 🔴 Tokenised by ci/check-abspath.php, not grepped. The grep version asked
+# whether the words appeared anywhere in the file, so a COMMENT saying the
+# guard was handled elsewhere passed it. Measured with a planted file whose
+# only mention was in a comment: "15 shipped PHP files examined, every one
+# guarded" — green over a file with no guard at all.
+#
+# The count and the floor stay: "every shipped file is guarded" is also what
+# zero shipped files prints, and shipped_files() swallows a dist-files.sh
+# failure with `|| true`. The floor is above 1 because a bad .distignore
+# pattern can collapse the list without emptying it.
+shipped_php=$(shipped_files)
+if [ -z "$shipped_php" ]; then
+  note_fail "No shipped PHP files were listed at all. The file list is wrong, so this check proved nothing."
+else
+  # shellcheck disable=SC2086
+  ci_php ci/check-abspath.php "--min=$MIN_SHIPPED_PHP_FILES" $shipped_php || fails=$((fails + 1))
 fi
 
 ci_step "no debug leftovers"