From 9b0b0ab4753087012ce01475e54fde7f89d926ef Mon Sep 17 00:00:00 2001 From: kristianwind Date: Thu, 17 Sep 2026 00:14:40 +0200 Subject: [PATCH] Make the CI checks say how many files they actually read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four checks in ci/ printed a reassuring line having examined nothing. Found independently by the woo-ai-manager and woo-cache-manager sessions in their own copies of this kit; all four reproduce here. A check that examined nothing prints the same sentence as one that examined everything, and only the passing side can tell them apart. That is why these survived being verified: a pattern that is too broad fails loudly the first time it meets real code, while one that finds nothing passes forever. So every success line now carries a count — the old text domain line could not tell 142 from 0. - static.sh, direct-access guard: counts what the loop examined and fails below MIN_SHIPPED_PHP_FILES. shipped_files() swallows a dist-files.sh failure with `|| true`, so an empty list is one broken .distignore away, and this is the check standing between a PHP file and being requested straight from a browser outside every capability check the code assumes has happened. The floor is above 1 because a pattern that wrongly excludes includes/ collapses the list without emptying it. - static.sh, credential scan: counts what it read, fatal on zero. A pushed key is compromised even after the commit is deleted, so a silent no-op here is not a weak check but the absence of one wearing a green tick. The floor is 1 and is not configurable: this list has no filter that could go partially wrong, so the only way it misreports is by coming out empty. - check-textdomain.php: three zeros, three verdicts. No PHP files at all is always fatal and deliberately not caller-configurable — it says the scan root is wrong, not that the plugin is small. Below --min is fatal. Zero calls with no --min is a warning, since a small plugin may legitimately have no translatable strings, but it is never silent. - check-textdomain.php: fix the annotated path. str_replace( $root, '', $path ) with the default root of "." removed every dot anywhere in the path, so a real error in includes/class-db.php was reported on "includes/class-dbphp" — not a file, not clickable, and not greppable. Every error this script printed with the default root was mangled that way. The floors live in ci/plugin.conf, the only file under ci/ that knows anything about this plugin, rather than hardcoded in static.sh: static.sh is copied verbatim between plugins too, so a number in it would make this plugin's shape everybody's constraint. ci_load_conf now requires them — an unset floor turns its comparison into a no-op and silently restores the bug. Each floor was broken once and watched to fail before being believed: empty shipped list, wrong scan root, --min above the real count, zero calls without --min, empty credential list, and a plugin.conf missing both floors. Static checks green, and now report 13 shipped PHP files, 142 translation calls across 17 files, 44 tracked files read. Co-Authored-By: Claude Opus 5 (1M context) --- ci/check-textdomain.php | 103 ++++++++++++++++++++++++++++++++++++---- ci/lib.sh | 5 +- ci/plugin.conf | 20 ++++++++ ci/static.sh | 39 +++++++++++++-- docs/REUSE.md | 20 +++++++- 5 files changed, 174 insertions(+), 13 deletions(-) diff --git a/ci/check-textdomain.php b/ci/check-textdomain.php index 8855f23..22ce345 100644 --- a/ci/check-textdomain.php +++ b/ci/check-textdomain.php @@ -8,17 +8,63 @@ * * Tokenised rather than grepped, so a domain-looking string inside a sentence * cannot pass for the real thing. + * + * 🔴 This script's whole job is to look at every string, which makes it the one + * check in the kit that can report success having found NOTHING. "Every + * translation call carries the text domain" is also exactly what a wrong scan + * root prints, or a refactor that moved the strings somewhere this does not + * walk. So it counts what it read and every success line carries the numbers: + * the old line could not tell 142 from 0. */ +// The floor is the caller's decision, because only the caller knows what zero +// would mean for its plugin. This script is copied between plugins and cannot +// know any of their sizes; a number hardcoded here would make one plugin's +// shape everybody's constraint. A plugin that has strings passes --min; one +// that legitimately has none leaves it off and gets a warning instead. +$min = null; +$args = array(); +foreach ( array_slice( $argv, 1 ) as $arg ) { + if ( 0 === strpos( $arg, '--min=' ) ) { + $min = (int) substr( $arg, 6 ); + continue; + } + $args[] = $arg; +} + // Copied between plugins, so there is no sensible default domain: a wrong one // would report "all good" while checking for somebody else's text domain. -if ( empty( $argv[1] ) ) { - fwrite( STDERR, "Usage: php ci/check-textdomain.php [root]\n" ); +if ( empty( $args[0] ) ) { + fwrite( STDERR, "Usage: php ci/check-textdomain.php [--min=N] [root]\n" ); exit( 1 ); } -$domain = $argv[1]; -$root = $argv[2] ?? '.'; +$domain = $args[0]; +$root = $args[1] ?? '.'; + +/** + * Path relative to the scan root, for the annotation. + * + * NOT str_replace( $root, '', $path ). With the usual root of "." that removes + * every dot ANYWHERE in the path, so a real error on "includes/class-db.php" + * was annotated on "includes/class-dbphp" — a filename that does not exist, is + * not clickable in a CI log, and does not match if you grep for it. Every + * error this script printed with the default root was mangled that way. + * + * @param string $path Path as the iterator produced it. + * @param string $root Scan root. + * @return string + */ +function ci_relative_path( $path, $root ) { + $root = rtrim( $root, '/' ); + + if ( '' !== $root && '.' !== $root && 0 === strpos( $path, $root . '/' ) ) { + return substr( $path, strlen( $root ) + 1 ); + } + + return ltrim( preg_replace( '#^\./#', '', $path ), '/' ); +} + $functions = array( '__', '_e', @@ -37,6 +83,8 @@ $functions = array( ); $problems = 0; +$calls = 0; +$files = 0; $iterator = new RecursiveIteratorIterator( new RecursiveCallbackFilterIterator( @@ -53,7 +101,8 @@ foreach ( $iterator as $file ) { continue; } - $path = $file->getPathname(); + $path = $file->getPathname(); + ++$files; $tokens = token_get_all( file_get_contents( $path ) ); $count = count( $tokens ); @@ -82,6 +131,8 @@ foreach ( $iterator as $file ) { continue; } + ++$calls; + // Collect every literal string passed at the top level of this call. $depth = 0; $strings = array(); @@ -110,7 +161,7 @@ foreach ( $iterator as $file ) { if ( ! in_array( $domain, $strings, true ) ) { printf( "::error file=%s,line=%d::The %s() call is missing the text domain '%s'.\n", - ltrim( str_replace( $root, '', $path ), '/' ), + ci_relative_path( $path, $root ), $token[2], $token[1], $domain @@ -121,9 +172,45 @@ foreach ( $iterator as $file ) { } if ( $problems > 0 ) { - printf( "\n%d translation calls are missing the text domain.\n", $problems ); + printf( "\n%d of %d translation calls are missing the text domain.\n", $problems, $calls ); exit( 1 ); } -echo "Every translation call carries the text domain '{$domain}'.\n"; +// Three different zeros, three different verdicts. This one is never +// legitimate and deliberately not caller-configurable: the smallest possible +// plugin still has its main file, so no PHP at all does not say "a small +// plugin", it says the ROOT IS WRONG and this check proved nothing. +if ( 0 === $files ) { + fwrite( STDERR, "No PHP files were found under '" . $root . "'. The scan root is wrong, so this check proved nothing.\n" ); + exit( 1 ); +} + +// The caller expected strings and there are none: the scan no longer finds +// this plugin's strings, whatever the reason. +if ( null !== $min && $calls < $min ) { + fwrite( + STDERR, + sprintf( + "Only %d translation call(s) across %d file(s), but at least %d were expected. The strings have moved somewhere this scan does not look, so this check proved nothing.\n", + $calls, + $files, + $min + ) + ); + exit( 1 ); +} + +// No floor was given, so zero may be honest — a genuinely small plugin can have +// no translatable strings. It must not be silent, though. +if ( 0 === $calls ) { + fwrite( STDERR, "Warning: no translation calls were found across " . $files . " file(s). Either this plugin has no translatable strings, or the scan missed them — pass --min=N to make that a failure.\n" ); + exit( 0 ); +} + +printf( + "All %d translation calls across %d files carry the text domain '%s'.\n", + $calls, + $files, + $domain +); exit( 0 ); diff --git a/ci/lib.sh b/ci/lib.sh index 1edb45a..4e0c7cc 100644 --- a/ci/lib.sh +++ b/ci/lib.sh @@ -18,8 +18,11 @@ ci_load_conf() { # shellcheck source=/dev/null . "$root/ci/plugin.conf" + # MIN_* included on purpose: a port that copies this file but forgets them + # leaves the floors empty, every comparison becomes a no-op, and the hollow + # checks these were added to fix come back silently — the same shape of bug. local missing= - for var in PLUGIN_SLUG PLUGIN_MAIN TEXT_DOMAIN NEEDS_WOOCOMMERCE; do + for var in PLUGIN_SLUG PLUGIN_MAIN TEXT_DOMAIN NEEDS_WOOCOMMERCE MIN_SHIPPED_PHP_FILES MIN_TRANSLATION_CALLS; do [ -n "${!var:-}" ] || missing="$missing $var" done if [ -n "$missing" ]; then diff --git a/ci/plugin.conf b/ci/plugin.conf index 2c464cc..e082941 100644 --- a/ci/plugin.conf +++ b/ci/plugin.conf @@ -31,3 +31,23 @@ NEEDS_WOOCOMMERCE=yes # create the data; the smoke files assert against it. A non-zero exit or an # uncaught error fails the build. SITE_TESTS="tests/fixtures.php tests/smoke.php" + +# Plausibility floors for the hygiene checks. They exist because a check that +# examined NOTHING prints the same reassuring line as one that examined +# everything, and only the passing side can tell them apart — a pattern that is +# too broad fails loudly on first contact with real code, one that finds nothing +# survives verification forever. +# +# Both are deliberately far below the real numbers. They are not assertions +# about this plugin's size: a tight number goes red on ordinary churn and gets +# raised until it means nothing. They assert that the scan still finds this +# plugin at all. +# +# Real counts when these were set (2026-09-17): 13 shipped PHP files, 142 +# translation calls across 17 files. Both floors were broken once and watched to +# fail before being believed. +MIN_SHIPPED_PHP_FILES=5 + +# Set to 0 for a plugin with no translatable strings: the check then warns +# instead of failing, so zero is never silent either way. +MIN_TRANSLATION_CALLS=60 diff --git a/ci/static.sh b/ci/static.sh index 9b5de8a..cb46a2e 100755 --- a/ci/static.sh +++ b/ci/static.sh @@ -54,12 +54,24 @@ 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)" -[ "$fails" = "$before" ] && echo " every shipped file is guarded" +# 🔴 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" +fi ci_step "no debug leftovers" if git ls-files -z '*.php' | xargs -0 grep -nE '\b(var_dump|print_r|dd)[[:space:]]*\(' ; then @@ -86,7 +98,12 @@ done < <(git ls-files -z '*.php') [ "$fails" = "$before" ] && echo " clean" ci_step "text domain" -ci_php ci/check-textdomain.php "$TEXT_DOMAIN" . || fails=$((fails + 1)) +# --min is deliberately far below the real count. It is not an assertion about +# how many strings this plugin has — a tight number goes red on ordinary churn +# and gets raised until it means nothing. It asserts the scan still finds this +# plugin's strings AT ALL, because "every translation call carries the text +# domain" is also what this prints having found none. +ci_php ci/check-textdomain.php "--min=$MIN_TRANSLATION_CALLS" "$TEXT_DOMAIN" . || fails=$((fails + 1)) ci_step "every file the plugin loads is a file the release contains" # The bug this exists for: a new class file is written, required from the main @@ -151,14 +168,30 @@ ci_step "no secrets in tracked files" # Deliberately narrow. Broad entropy heuristics fire on minified assets, hashes # and base64 images, and a check that cries wolf gets commented out — which is # worse than not having it. These are prefixes that mean one thing only. +# +# 🔴 And it says how many files it read. A pushed key is compromised even after +# the commit is deleted, so this scan silently examining nothing is not a weak +# check — it is the absence of one, wearing a green tick. +# +# The floor here is 1 and is NOT plugin-configurable, unlike the shipped-file +# one above: this list is every tracked file with no filter that could go +# partially wrong, so the only way it misreports is by coming out empty, and +# any higher number would just be a guess about the plugin's size. The count on +# the success line is what catches a list that shrank without emptying. before=$fails +secrets_scanned=0 while IFS= read -r -d '' f; do [ -f "$f" ] || continue + secrets_scanned=$((secrets_scanned + 1)) if LC_ALL=C grep -nE '(sk-ant-[A-Za-z0-9_-]{16,}|sk-[A-Za-z0-9]{32,}|ghp_[A-Za-z0-9]{30,}|gho_[A-Za-z0-9]{30,}|github_pat_[A-Za-z0-9_]{50,}|AKIA[0-9A-Z]{16}|-----BEGIN [A-Z ]*PRIVATE KEY-----)' "$f"; then note_fail "$f looks like it contains a credential. It cannot be unpushed — rotate it." fi done < <(git ls-files -z) -[ "$fails" = "$before" ] && echo " nothing that looks like a key or a private key" +if [ "$secrets_scanned" -lt 1 ]; then + note_fail "The credential scan examined no files at all. git tracks nothing here, so this check proved nothing." +elif [ "$fails" = "$before" ]; then + echo " $secrets_scanned tracked files read, nothing that looks like a key or a private key" +fi ci_step "no placeholders left in the plugin header" # The header is the plugin's business card: wp-admin shows the author and the diff --git a/docs/REUSE.md b/docs/REUSE.md index 8dd403c..a525cbe 100644 --- a/docs/REUSE.md +++ b/docs/REUSE.md @@ -20,7 +20,7 @@ ci/ every script; plugin.conf is the only one you edit .distignore ``` -**2. Rewrite `ci/plugin.conf`** — five values, all commented in the file: +**2. Rewrite `ci/plugin.conf`** — seven values, all commented in the file: ```sh PLUGIN_SLUG=your-plugin @@ -28,8 +28,26 @@ PLUGIN_MAIN=your-plugin.php TEXT_DOMAIN=your-plugin NEEDS_WOOCOMMERCE=yes # no, for a plugin that does not need it SITE_TESTS="tests/fixtures.php tests/smoke.php" +MIN_SHIPPED_PHP_FILES=5 # floors: see below +MIN_TRANSLATION_CALLS=60 # 0 for a plugin with no translatable strings ``` +`ci_load_conf` fails if any of them is unset, the two floors included — an +empty floor turns its comparison into a no-op and quietly restores the bug the +floors exist for. + +Set each floor well below the plugin's real count. They are not assertions +about size: a tight number goes red on ordinary churn and gets raised until it +means nothing. They assert that the scan still finds the plugin at all, because +**a check that examined nothing prints the same reassuring line as one that +examined everything**. Count the real numbers first (`./ci/dist-files.sh | grep +-c '\.php$'`, and the tail of a `ci/static.sh` run), then take roughly a third. + +Then break each floor once and watch it fail before believing it. A pattern +that is too broad fails loudly the first time it meets real code; one that is +too narrow is indistinguishable from one that finds nothing, and passes +forever — which is why every success line in this kit now carries a count. + **3. Adjust `.distignore`** for that plugin's internal documents. Everything left in `wp-content/plugins/` can in principle be fetched over HTTP from a live shop, so notes and tooling stay out of the zip.