Merge pull request #122 from Hypercart-Dev-Tools/development

Development to Main

Author: noelsaw1

4X4.md

@@ -0,0 +1,103 @@
+---
+client: Hypercart / Neochrome
+repo: https://github.com/Hypercart-Dev-Tools/WP-Code-Check.git
+last_edit: 2026-03-23
+week_of: 2026-03-23
+source_pr_number: n/a
+sprint: planning-refresh
+
+---
+
+Pro-tip: Ask your VS Code AI to self populate the metadata above.
+
+# 4x4 Dashboard - Strategic Goals & Weekly Checklist
+A simple, actionable framework to prioritize and track engineering tasks. Focus on alignment, transparency, continuous improvement, and increasing clarity for everyone. This document does not replace your project management tools. It is meant to be a simple, actionable checklist to help you focus on the most important tasks for the week.
+
+# About the Current Project
+WP Code Check is a zero-dependency static analysis toolkit for WordPress performance, security, and reliability issues. Current engineering focus is stabilizing the search backend, reducing false positives in noisy direct-pattern rules, restoring fixture parity, and creating a measured path for an optional Semgrep backend without destabilizing the Bash scanner.
+
+---
+
+## 1. Strategic Backlog
+**Maximum of 4 items. Focus on long-term goals and impactful improvements.**
+**Update/Reminder:** If these are new projects without a clear scope, consider using the [Project Scope Outline](#bonus-project-scope-outline) below.
+
+1. - [ ] Unify search backends under one wrapper layer - Replace timeout-wrapped raw file-discovery calls and helper fallbacks with a single API for file discovery, line matches, and context extraction.
+2. - [ ] Restore fixture and scorecard trust - Audit failing fixtures, align expected counts with intended semantics, and make regression output credible before larger rule migrations.
+3. - [ ] Pilot Semgrep on the highest-noise direct rules - Start with `unsanitized-superglobal-read`, `unsanitized-superglobal-isset-bypass`, `wpdb-query-no-prepare`, and `file-get-contents-url` using side-by-side scorecards.
+4. - [ ] Continue rule quality cleanup in Bash - Keep reducing false positives where the current engine is already close, especially heuristics and context-sensitive detectors that do not map cleanly to Semgrep.
+
+---
+
+## 2. Current Week
+**Active tasks for the week. Maximum of 4 items.**
+
+> **Tip:** If your team frequently handles urgent issues, consider reserving 1-2 slots for hotfixes. Otherwise, use all 4 slots for planned work.
+
+- [x] Refresh planning source of truth - Updated the Semgrep migration plan to match the current codebase, deprecated `BACKLOG.md`, and moved active planning into this 4X4.
+- [ ] Add file-discovery wrapper spike - Draft `cached_file_search()` or equivalent and replace one or two high-value call sites such as `AJAX_FILES` and `TERMS_FILES` to prove the interface. Benefit: fewer one-off search code paths, easier maintenance, and a lower chance that one slow check behaves differently from the rest.
+- [ ] Add observability for slow checks - Implement per-check timeout warnings and a small top-N slow-check summary so long scans stop looking stuck. Benefit: users can see what is slow, what timed out, and whether the scanner is still making progress instead of guessing that it froze.
+- [ ] Audit fixture mismatches and shortlist Semgrep pilots - Re-run failing fixtures, classify false positives vs desired behavior, and finalize the first four Semgrep scorecard candidates. Benefit: more trustworthy test results, clearer migration decisions, and less risk of moving a noisy or inaccurate rule to a new backend.
+
+---
+
+## 3. Previous Week
+**Review completed, deferred, or blocked tasks from the prior week.**
+
+- [x] Added Path B observability for aggregated magic-string patterns - phase timing and quality counters are now visible in text and JSON output.
+- [x] Fixed stale-registry fallback behavior - eliminated one apparent hang path in the pattern loader and guarded empty search patterns.
+- [x] Fixed high-noise direct-pattern false positives - reduced `php-shell-exec-functions`, `spo-002-superglobals`, and `php-dynamic-include` noise with targeted scanner and pattern fixes.
+- [ ] Phase 0b observability remains incomplete - heartbeat output and slow-check rollups are still deferred and need a focused pass.
+
+---
+
+## 4. Recent Lessons Learned
+**Capture insights to improve processes and avoid repeating mistakes.**
+
+1. Small search-path inconsistencies create outsized noise - most recent false positives came from runner behavior and shell quoting, not from the pattern ideas themselves.
+2. Timeout protection is necessary but not sufficient - a timed-out check that silently passes prevents hangs, but it also hides diagnostic value unless we surface warnings and timing data.
+3. Planning drift happens fast in a monolithic script - line-number-based roadmap docs stale quickly, so strategic docs need periodic refreshes tied to current code references.
+4. Not every noisy rule is a Semgrep problem first - if a Bash rule is already close after a targeted fix, it should drop in Semgrep priority behind noisier or structurally simpler candidates.
+
+---
+
+## Bonus: Project Scope Outline
+
+> **Note:** This section is a work-in-progress and is being developed as a natural extension of the 4x4 methodology. It is not yet a core part of the framework but is included here as a supplementary tool for teams who want to add more context to their planning.
+
+### 1. Goals
+Keep WP Code Check fast, explainable, and operationally reliable while improving detection quality on high-value WordPress performance and security checks.
+
+### 2. Assumptions
+- The current Bash scanner remains the default engine through any Semgrep pilot.
+- Search backend stabilization should land incrementally rather than via a large rewrite.
+- Fixture parity work is required before scorecards will be trusted for migration decisions.
+- The highest-value weekly work is small and verifiable: one wrapper spike, one observability pass, one fixture audit pass.
+
+### 3. Potential Risks
+- Semgrep rules may look cleaner on paper but still miss WordPress-specific context that the Bash pipeline currently encodes.
+- Wrapper changes can improve maintainability while accidentally changing output parity if they are not backed by fixture comparisons.
+- Monolithic-script edits can create regression risk in unrelated checks unless changes stay narrow and verified.
+- Planning can split across too many docs unless this file stays the active weekly view.
+
+### 4. Long-Term Maintainability
+Long-term sustainability depends on shrinking the amount of bespoke search behavior in `check-performance.sh`, keeping rule logic externalized where possible, maintaining trustworthy fixtures, and promoting only the rules to Semgrep that clearly beat the Bash implementation on quality and runtime.
+
+---
+
+## How to Use This Template
+
+For detailed guidance on the 4x4 framework, see the [README](README.md).
+
+**Quick tips:**
+- Update this document weekly (move "Current Week" to "Previous Week" and plan your new week)
+- Limit each section to 4 items maximum to maintain focus
+- Capture lessons learned immediately while they're fresh
+- Link to detailed tasks in your project management tools (GitHub, Jira, etc.)
+
+---
+
+## License
+This work is licensed under the Creative Commons Attribution 4.0 International (CC BY 4.0) License. To view a copy of this license, visit [https://creativecommons.org/licenses/by/4.0/](https://creativecommons.org/licenses/by/4.0/).
+
+Copyright © 2026 Hypercart DBA Neochrome, Inc. | 4x4Clarity.com

CHANGELOG.md

@@ -22,6 +22,11 @@ All notable changes to this project will be documented in this file.
 
 ### Documentation
 
+- Refreshed planning docs to match the current codebase and consolidated active planning into `4X4.md`
+  - Updated `PROJECT/1-INBOX/FEATURE-SEMGREP-MIGRATION-PLAN.md` with current search-backend hotspots, refreshed Phase 0 status notes, and reprioritized the Semgrep pilot list based on current false-positive pressure
+  - Deprecated `PROJECT/2-WORKING/BACKLOG.md` as an active planning source and converted it into a pointer to `4X4.md` plus the Semgrep roadmap doc
+  - Populated `4X4.md` with current strategic goals, weekly work, prior-week accomplishments, and planning assumptions for WP Code Check
+
 - Added `PROJECT/1-INBOX/PATTERN-PROPOSAL-LAUNCHPAD-CRASH.md`
   - Captures the plan for converting the Launchpad crash lessons into generalized WPCC anti-pattern proposals
   - Recommends against a single environment-specific "crash detector"

FEEDBACK-CR-SELF-SERVICE.md

@@ -9,37 +9,43 @@
 
 ### ✅ Fix Now — High Confidence, Low Effort
 
-- [ ] **FIX `php-shell-exec-functions.json` — `exec-call` pattern matches `curl_exec()`**  
+- [x] **FIX `php-shell-exec-functions.json` — `exec-call` pattern matches `curl_exec()`** ✅ *Fixed in commit 740ba08*  
   **Pattern:** `exec[[:space:]]*\(` has no word boundary → matches `curl_exec(`.  
   **Fix:** Change to `\bexec[[:space:]]*\(` in the `exec-call` sub-pattern.  
   **File:** `dist/patterns/php-shell-exec-functions.json`  
   **FPs eliminated:** 8 (all CRITICAL — all were `curl_exec($curl)` calls)
 
-- [ ] **FIX `php-dynamic-include.json` — WP-CLI bootstrap scripts flagged as LFI**  
+- [x] **`php-dynamic-include.json` — WP-CLI bootstrap scripts no longer flagged as LFI** ✅ *Resolved in follow-up commit*  
   **Finding:** `check-user-meta.php:13` and `test-alternate-registry-id.php:24` — `$path` is iterated from a hardcoded static array, never user-controlled.  
-  **Fix:** Add `*-user-meta.php`, `*-registry-id.php`, or more broadly `*/scripts/*` / `*/cli/*` to `exclude_files` in the pattern.  
-  **File:** `dist/patterns/php-dynamic-include.json`  
+  **Attempted fix (740ba08 — insufficient):** Added `wp-load` to `exclude_patterns`, but the actual matched line is `require_once $path;` — it does not contain `wp-load`.  
+  **Proper fix:** Added new `exclude_if_file_contains` capability to the simple pattern runner and `dist/bin/check-performance.sh`. When a matched file's content contains any string listed in the new `exclude_if_file_contains` JSON array, all matches in that file are suppressed. Added `"wp eval-file"` to `php-dynamic-include.json` under this key — both WP-CLI scripts have this string in their docblock comment.  
+  **Files changed:** `dist/bin/check-performance.sh` (runner feature), `dist/patterns/php-dynamic-include.json` (new exclusion key)  
   **FPs eliminated:** 2 (both CRITICAL)
 
 ---
 
-### 🔍 Investigate Before Acting — Diagnosis Uncertain
-
-- [ ] **INVESTIGATE "Direct superglobal manipulation" (~17 findings on `CURLOPT_POST`)**  
-  **Reviewer claim:** `curl_setopt($curl, CURLOPT_POST, true)` is matched as superglobal manipulation.  
-  **Our assessment:** The `spo-002-superglobals` pattern requires `$_` prefix in all branches — `CURLOPT_POST` cannot match it.  
-  **Action:** Re-examine the actual line numbers in the scan log for these 17 findings. Determine which rule is actually firing and why. Do NOT apply the reviewer's suggested fix (`$_` anchoring) — it's already implemented.  
-  **File:** `dist/patterns/spo-002-superglobal-manipulation.json` (likely not the culprit)
-
-- [ ] **INVESTIGATE "Sanitized reads flagged as unsanitized" for `sanitize_text_field($_GET[...])`**  
-  **Finding:** `class-cr-rest-api.php` and `class-cr-business-rest-api.php` — `sanitize_text_field($_GET['registry_id'])` being flagged.  
-  **Our assessment:** `sanitize_` is already in `exclude_patterns` for `unsanitized-superglobal-read`. This is likely a **multiline case** — `$_GET` on its own line while the sanitizer wraps on another.  
-  **Action:** Confirm by inspecting actual flagged lines. If multiline, document as a known structural limitation (grep is line-scoped; lookbehinds won't help here). If same-line, there's a bug in the exclude logic.  
-  **File:** `dist/patterns/unsanitized-superglobal-read.json`
+### ✅ Implemented After Investigation
+
+- [x] **FIX `spo-002-superglobals` inline grep corruption** ✅ *Implemented in scanner*  
+  **Scan log findings:** 31 total spo-002 findings. 16 are `CURLOPT_POST`/`CURLOPT_POSTFIELDS`, 2 are JS `type: 'POST'` strings, 4 are `$_SERVER` reads (SERVER not in the pattern alternation), 1 is the only legitimate finding (line 1014).  
+  **Root cause confirmed:** The inline bash spo-002 grep (check-performance.sh ~line 3723) uses a **double-quoted string with `\\$_`**. In bash double-quotes, `\\` → `\` and then `$_` starts expansion of the bash `$_` special variable (last argument of the previous command). At runtime, `$_` contains the last argument from `text_echo "▸ Direct superglobal manipulation..."` — an ANSI-coloured string including `[HIGH]`. This corrupts the entire ERE pattern, causing it to match incorrectly in a non-deterministic way.  
+  **The JSON pattern itself (`\$_(GET|POST...)`) is correct** — tested via `load_pattern` + direct grep, it does NOT match CURLOPT_POST. The bug is entirely in the inline bash code, not the JSON pattern file.  
+  **Fix implemented:** Changed the inline grep at line 3723 from double-quoted to single-quoted string, which prevents `$_` expansion. This is a **scanner bug, not a pattern bug**. The JSON file did not need to change.  
+  **File to fix:** `dist/bin/check-performance.sh` ~line 3723  
+  **Verified impact:** `spo-002-superglobals` dropped from **31 → 3** findings in the follow-up scan. Remaining 3 are legitimate review cases: `$_POST['force_refresh']`, `unset($_GET['activate'])`, and `$_GET['view_errors']` conditional logic.
+
+- [x] **FIX simple runner ignoring `exclude_patterns` / `exclude_files`** ✅ *Implemented in scanner*  
+  **Scan log findings:** 30 `unsanitized-superglobal-read` findings. Confirmed FPs include: `class-cr-rest-api.php:90`, `class-cr-rest-api.php:98`, `class-cr-rest-api.php:843`, `class-cr-business-rest-api.php:103`, `class-cr-business-rest-api.php:138`, `class-cr-business-rest-api.php:857` — all are same-line ternary patterns like `isset($_GET['x']) ? sanitize_text_field($_GET['x']) : ''`.  
+  **Root cause confirmed:** The simple pattern runner (`check-performance.sh` ~line 5970) runs `cached_grep -E "$pattern_search"` but **never applies `exclude_patterns` from the JSON definition**. The `exclude_patterns` array in `unsanitized-superglobal-read.json` (which includes `sanitize_`, `isset\(`, `esc_`, etc.) is loaded but silently ignored. The legacy inline checks manually pipe through `grep -v` to apply exclusions; the JSON-driven simple runner does not.  
+  **This is NOT a multiline issue** — the flagged lines all have the sanitizer wrapper on the same line. The exclusion simply isn't being applied at all by the simple runner.  
+  **Additional FPs from same root cause:** `clear-person-cache.php:34`, `setup-user-registry-id.php:23-24`, `set-account-type.php:26-27` — all properly guarded `$_POST` reads with nonce verification on the same line.  
+  **Fix implemented:** The simple pattern runner now parses both `exclude_patterns` and `exclude_files` from the JSON pattern file and filters matches before JSON findings are added. This improves behavior across all JSON-defined `grep`/`simple` patterns, not just `unsanitized-superglobal-read`.  
+  **File to fix:** `dist/bin/check-performance.sh` ~line 5970 (simple pattern runner grep call)  
+  **Verified impact:** `unsanitized-superglobal-read` dropped from **30 → 19** findings in the follow-up scan. The remaining 19 are mostly other classes of reads that still require separate tuning, especially the dedicated `unsanitized-superglobal-isset-bypass` rule.
 
 ---
 
-### 📋 Deferred — Valid Issues, Structural Effort Required
+### 📋 Deferred — Investigate Further Before Implementing
 
 - [ ] **DEFERRED: Add admin-only hook whitelist for capability check false positives**  
   **Finding:** `credit-registry-forms.php:48` — `add_action('admin_notices', ...)` flagged for missing capability check. `admin_notices` only fires for authenticated admin users.  
@@ -81,11 +87,13 @@
 
 ## Impact Summary
 
-| Fix | File to Edit | Effort | FPs Eliminated |
-|-----|-------------|--------|---------------|
-| `\b` word boundary on `exec-call` | `php-shell-exec-functions.json` | 1 line | 8 |
-| Add WP-CLI scripts to `exclude_files` | `php-dynamic-include.json` | 2 lines | 2 |
-| Investigate superglobal 17-finding cluster | Scan log + `spo-002` | Investigation | Up to ~17 |
-| Investigate multiline sanitizer FPs | Scan log + `unsanitized-superglobal-read` | Investigation | Up to ~20 |
-| Admin-only hook whitelist | `check-performance.sh` | Medium | 1+ per scan |
-| N+1 loop containment tightening | `check-performance.sh` | Medium | 2+ per scan |
+| Fix | File to Edit | Effort | FPs Eliminated | Status |
+|-----|-------------|--------|---------------|--------|
+| `\b` word boundary on `exec-call` | `php-shell-exec-functions.json` | 1 line | 8 | ✅ Done (740ba08) |
+| `exclude_if_file_contains` + `wp eval-file` | `check-performance.sh` + `php-dynamic-include.json` | Medium | 2 verified | ✅ Done |
+| Single-quote inline spo-002 grep | `check-performance.sh` ~L3723 | 1 line | 28 verified | ✅ Done |
+| Apply `exclude_patterns` in simple runner | `check-performance.sh` ~L5970 | Medium | 11 verified | ✅ Done |
+| Admin-only hook whitelist | `check-performance.sh` | Medium | 1+ per scan | 📋 Deferred |
+| N+1 loop containment tightening | `check-performance.sh` | Medium | 2+ per scan | 📋 Deferred |
+
+**Latest measured totals:** 99 findings before scanner fixes → **88 findings after first round** → **86 findings after dynamic-include fix**.