Run Plugin Check in CI, clear its 70 errors, and guard outbound HTTP statically (P0-4, P0-10)

[Shubochandrosarker]

What this changes

Three things this tree is missing, and they belong together: without the CI job the fixes cannot be verified here, and the guard is the static half of the same network-silence promise the fixes touch.

1. The plugin-check job — P0-4's first acceptance criterion. Blocking, checking the distributable tree built with bin/build-dist.sh.
2. The errors it reports — 70 on the working copy's equivalent tree, cleared. Nothing silenced.
3. OutboundHttpAllowlistTest — P0-10's third criterion, the static half of invariant I5.

Replaces #7, which I opened and closed: I built that on a stale view of main from the start of the session, before #3–#6 landed here, so it duplicated the convergence through a divergent history and conflicted. This one branches from current main and contains only what is genuinely absent.

Advances P0-4 and P0-10.

Type

• Bug fix
• Feature
• Refactor (no behaviour change)
• Documentation
• Security fix
• Release/CI plumbing

Why these three together

They are one outcome — "Plugin Check runs here and passes" — split across a job, the fixes it demands, and a guard for the invariant two of those fixes touch. Landing the job alone turns CI red on main; landing the fixes alone leaves them unverified in this repository, which is exactly what the provenance rule on the backlog page warns against. Happy to split if you would rather review them separately.

The 70, by category

i18n — 44. Forty-one __() / esc_html__() / _n() calls with placeholders had no translators: comment, and three strings used bare %s/%d twice, leaving a translator unable to reorder them for languages that need different word order. The comments say what each placeholder actually is — %s: string would satisfy the sniff and help nobody.

Five had to be wrapped in <?php ?>. They sit in inline HTML in the corporate module, where a bare /* ... */ line is not a comment — it is text, and it would have rendered on the group admin screens. I introduced that with a bulk insertion and caught it by tokenising each file and asserting every translators: line is a real T_COMMENT, not by reading the diff. Worth knowing if you add more.

Escaping — 15. Ten are the booking CSS prefix in templates/account.php, already reduced to [a-z0-9_-] by a preg_replace() fifteen lines above the output. esc_attr() is a genuine no-op on that value; it is added so the escaping is visible where the output happens, with a comment recording why. Five are real: two unescaped counts in a corporate admin notice, and three unescaped wp_die() arguments in the Stripe service, including a link_url.

Filesystem — 10. Two were already decided and merely mis-suppressed: the readfile() calls carried phpcs:ignore comments naming file_system_read_readfile, but WPCS renamed the sniff to file_system_operations_readfile, so they matched nothing and the calls were reported anyway. The sibling in class-documents.php uses the current code and was never reported — that mismatch is what exposed it. A stale sniff code is a silent suppression failure; nothing warns you. The other eight are two php://output export streams (the HTTP response body, not a file) and two streaming CSV readers. WP_Filesystem has no streaming API and no CSV parser, and get_contents_array() splits on newlines, which corrupts any quoted field containing one — member notes and waiver text both do. Suppressed per line with the reason, the way the ~50 UnescapedDBParameter cases already are.

Heredoc — 1. admin_css() returned a 79-line CSS heredoc. The body has no $, backslash or single quote, so the delimiter swap needs no escaping.

One deliberate, verified behaviour delta

The heredoc conversion is not byte-identical. A quoted string includes the newlines adjacent to its quotes; a heredoc excludes the one before its terminator. Verified by hashing both literals in isolation:

heredoc version : 29635a0e862d33386b50561995322803  8858 bytes
string  version : ea26e9cfe34e6d85836c52bce9bc3549  8860 bytes
relationship    : new === "\n" . old . "\n"

It is enqueued as an inline stylesheet, where leading and trailing whitespace is inert. Flagging it rather than claiming identity.

Rules touched

• PMPro runtime independence
• No seeded/priced plans on fresh install
• Entitlements fail closed
• A lapsed licence never breaks member access
• No outbound HTTP on fresh activation
• Every REST route has a real permission_callback
• None of the above

Strengthened, not relaxed. FreshInstallTest already proves activation and init reach the network zero times, but only for the paths those two moments execute — a wp_remote_post() added behind admin_init, a shortcode, a REST callback or a cron handler leaves that suite green and still breaks the promise. The new guard allow-lists the files permitted to call out against the integration key gating them (two here, coreSTORE and Stripe, both 'no'), checks the list in both directions so an entry whose call disappears must be removed rather than left as a standing permission, and asserts the gating toggle ships off rather than merely that the file is listed. Matching is token_get_all(), not a regex: a regex for wp_remote_post also matches the phpdoc describing it, which fills the allow-list with files that never call anything and hides the one that does.

Multi-edit checklist

• N/A — no new production classes, no schema change, no new shortcode, no new capability. MEMBERISTIC_DB_VERSION untouched, no version bump: this is not a release PR.

Documentation

• No docs needed, because no hook, integration, entitlement, shortcode or REST route changed behaviour, and nothing here is user-visible. CHANGELOG.md and readme.txt are untouched for the same reason.

The backlog is deliberately not ticked in this commit. The provenance rule on that page says a box is ticked only when the evidence is in this repository and its CI is green. Plugin Check has never run here, so I cannot honestly tick P0-4 before this PR's own job reports. I will push the backlog update once it is green — or fix whatever it finds first.

Follow-up not included: four translatable strings changed msgid (the placeholder reordering). Only languages/memberistic.pot ships — no compiled .po/.mo — so no translation is lost, but the .pot is stale for those four entries and lacks the new translator comments. Regenerating needs WP-CLI (wp i18n make-pot).

What I ran

Against this tree, not the working copy:

$ find . -name '*.php' -not -path './vendor/*' -print0 | xargs -0 -n1 php -l
(no output — clean)

$ find assets -name '*.js' -print0 | xargs -0 -n1 node --check
(no output — 11 files clean)

$ phpunit -c phpunit.xml
OK (56 tests, 856 assertions)

The guard test passing here matters: its allow-list is derived from the working copy, and this confirms this tree has the same two outbound call sites and no others.

Both new guards were mutation-tested in both directions rather than trusted because they were green:

# wp_remote_get() added to an unlisted shipped file
  - includes/utilities/helpers.php — wp_remote_get() on line 583   → FAIL

# coreSTORE flipped to 'default' => 'yes'
  ...reaches an off-site service, but integration 'corestore' ships enabled.  → FAIL

On the mirror, where the job already exists, this same set of fixes took Plugin Check from 70 errors to 0 with warnings unchanged at 266 — that pairing is the evidence the errors were fixed rather than reclassified.

What I did NOT test

• Plugin Check on this tree. It needs a real WordPress and Composer, neither available in my environment. This PR's own job is the verification. This tree is 2.0.1 and differs from the mirror, so it may surface errors the mirror did not; if so they are mine to fix and I will iterate on this branch.
• The ci.yml YAML parse. No python+yaml available locally. Structurally checked (job key at the same indentation as its siblings, no tabs), but GitHub is the real parser.
• The corporate admin screens visually. Five comments moved inside <?php ?> in inline HTML, and ?> swallows the following newline, so the emitted markup shifts by a newline in five places. Inert in HTML, not eyeballed.
• The CSV importers end to end — only comments and annotations changed there, but no import was run.

Risk and rollback

• Blast radius if this is wrong: the CI job and the test carry no runtime risk. The production surface is the P0-4 diff: three wp_die() argument escapes, two integer casts, a CSS literal delimiter change, and comment/annotation additions. The realistic failure mode is a display regression on the corporate group screens, not data loss. The one new blocking job could turn main red if it behaves differently here than on the mirror — that is the point of it, but it is the change most likely to need a follow-up.
• How to roll it back: revert the merge commit, or drop the plugin-check job alone if only the gate is unwanted.
• Does it need a data migration to reverse? no

🤖 Generated with Claude Code