ci: harden pre-merge checks and supply chain - #19
Conversation
- pin all GitHub Actions to commit SHAs across every workflow - add commitlint job validating PR commits (Conventional Commits) - add svelte-check type-check gate for editor-app and admin-app - add PHPCS (WordPress security/correctness ruleset) and PHPStan (level 5 with baseline) for the plugin PHP, replacing php -l alone - add npm audit job (root + both Svelte apps, high/critical gate) - add CI concurrency to cancel superseded PR runs - fix mispositioned phpcs:ignore in class-rebemer-rest.php so the safe $wpdb->prepare() IN(...) query passes the new PreparedSQL sniff Co-authored-by: Jack Granatowski <contact@codeslash.net>
|
Warning Review limit reached
More reviews will be available in 11 minutes and 58 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR establishes comprehensive code quality and static analysis infrastructure for the SLASHED plugin suite. It hardens GitHub Actions workflows against supply-chain attacks by pinning all third-party actions to specific commit SHAs, adds npm audit checks to catch high-severity dependency vulnerabilities, introduces PHP CodeSniffer and PHPStan static analysis with appropriate configuration, configures Svelte type-checking for both Bricks app variants, and applies widespread code formatting improvements to align with new tooling standards. ChangesCode Quality and Tooling Infrastructure
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
PHPCS now enforces the complete WordPress-Extra ruleset (was a curated security/correctness subset): - phpcbf-formatted all PHP to WP conventions (tabs, array layout, spacing, pre-increment, single quotes) — whitespace/format only, no logic change - rename $_slashed_shared -> $slashed_shared so the bootstrap global is properly prefixed (PrefixAllGlobals) - configure i18n sniff for the three real text domains (slashed, slashed-bricks, slashed-gutenberg) matching the standalone-plugin layout - document-justified exclusions: class file-name convention, unused params in WP hook callbacks; advisory warnings stay non-blocking PHPStan: shrink the baseline from 71 to 9 by fixing root causes instead of freezing noise: - phpstan-bootstrap.php declares the runtime define() constants - ignore dynamic require_once path resolution - treatPhpDocTypesAsCertain: false (PHPDoc types aren't runtime-enforced)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
phpstan-baseline.neon (1)
12-16: Note: Unreachable code warnings in baseline.The baseline suppresses 4 "unreachable code" findings in
class-framework-updater.php. These typically indicate dead code paths that could be removed in a follow-up cleanup. The baseline approach is appropriate for this PR's goal of establishing tooling gates without blocking on pre-existing issues.Would you like me to investigate the unreachable code paths and suggest fixes for a follow-up PR?
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@phpstan-baseline.neon` around lines 12 - 16, The baseline currently suppresses four dead-code (deadCode.unreachable) findings for SLASHED-for-WP/includes/class-framework-updater.php; identify the unreachable branches inside class Framework_Updater (methods likely containing early returns/exits) by running phpstan on that file and locating the specific unreachable statement sites, then in a follow-up PR either remove the dead branches or refactor control flow to make the code reachable (e.g., consolidate early returns, remove redundant code after exit/throw, or guard branches appropriately), and update or remove the specific baseline entries once the unreachable statements are fixed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 35-38: The release workflow currently enables npm caching in the
actions/setup-node step (the step using
actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e and node-version:
22); remove the cache: npm entry from that step so dependencies are fetched
fresh during releases, and verify there are no other npm cache entries in the
same workflow to ensure reproducible, secure release builds.
---
Nitpick comments:
In `@phpstan-baseline.neon`:
- Around line 12-16: The baseline currently suppresses four dead-code
(deadCode.unreachable) findings for
SLASHED-for-WP/includes/class-framework-updater.php; identify the unreachable
branches inside class Framework_Updater (methods likely containing early
returns/exits) by running phpstan on that file and locating the specific
unreachable statement sites, then in a follow-up PR either remove the dead
branches or refactor control flow to make the code reachable (e.g., consolidate
early returns, remove redundant code after exit/throw, or guard branches
appropriately), and update or remove the specific baseline entries once the
unreachable statements are fixed.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 37d19ddc-87fb-4787-942c-4538954a9d9e
⛔ Files ignored due to path filters (3)
SLASHED-for-WP/integrations/bricks/admin-app/package-lock.jsonis excluded by!**/package-lock.jsonSLASHED-for-WP/integrations/bricks/editor-app/package-lock.jsonis excluded by!**/package-lock.jsoncomposer.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/framework-sync.yml.github/workflows/release.yml.gitignoreSLASHED-for-WP/includes/class-admin.phpSLASHED-for-WP/includes/class-color-resolver.phpSLASHED-for-WP/includes/class-css-generator.phpSLASHED-for-WP/includes/class-framework-updater.phpSLASHED-for-WP/includes/class-hooks-page.phpSLASHED-for-WP/includes/class-inventory.phpSLASHED-for-WP/includes/class-rest-controller.phpSLASHED-for-WP/includes/class-settings.phpSLASHED-for-WP/includes/class-token-defaults.phpSLASHED-for-WP/includes/class-token-page.phpSLASHED-for-WP/integrations/bricks/admin-app/jsconfig.jsonSLASHED-for-WP/integrations/bricks/admin-app/package.jsonSLASHED-for-WP/integrations/bricks/editor-app/jsconfig.jsonSLASHED-for-WP/integrations/bricks/editor-app/package.jsonSLASHED-for-WP/integrations/bricks/includes/class-classes.phpSLASHED-for-WP/integrations/bricks/includes/class-enqueue.phpSLASHED-for-WP/integrations/bricks/includes/class-rebemer-rest.phpSLASHED-for-WP/integrations/bricks/includes/class-variables.phpSLASHED-for-WP/integrations/bricks/slashed-bricks.phpSLASHED-for-WP/integrations/gutenberg/includes/class-editor-enqueue.phpSLASHED-for-WP/integrations/gutenberg/includes/class-presets.phpSLASHED-for-WP/integrations/gutenberg/slashed-gutenberg.phpSLASHED-for-WP/slashed.phpcomposer.jsonphpcs.xml.distphpstan-baseline.neonphpstan-bootstrap.phpphpstan.neon.dist
Resolve the leftover lint/analysis findings at the source so PHPStan now runs with no baseline (fully strict) and PHPCS reports zero warnings: - fix malformed @Property PHPDoc that broke the doc parser - replace array_filter($parts, 'strlen') with a boolean callback - guard CSS reads with is_readable() and drop the @ error suppression; justify file_get_contents on a local file - remove redundant array_values() on already-list values - remove unreachable return; after wp_send_json_* (which always exit) - reword comments that tripped the commented-out-code heuristic - inline @PHPStan-Ignore for the legitimate standalone-mode method_exists guard, with an explanatory comment - phpcs.xml.dist: recognise Bricks' bricks_full_access capability; silence false-positive dynamic-hook-name and reserved-keyword ($var) advisories
Address CodeRabbit/zizmor cache-poisoning finding: a poisoned dependency cache could be baked into the published plugin zip. Release builds now fetch dependencies fresh from the registry with integrity verification. CI workflows keep their cache (they don't publish artifacts).
This pull request was created by @kiro-agent on behalf of @jackgranatowski 👻
Comment with /kiro fix to address specific feedback or /kiro all to address everything.
Learn about Kiro Web
Summary
Closes the gaps in the plugin's pre-merge gate: real PHP analysis, frontend type-checking, commit validation, dependency auditing, and supply-chain pinning.
php -lsyntax check alone:WordPress.Security,WordPress.DB, deprecated/enqueue sniffs,PHPCompatibilityWPfor the PHP 7.4 floor). Advisory warnings don't fail the build; errors do.szepeviktor/phpstan-wordpress) and a baseline freezing the 71 pre-existing findings, so the gate catches new issues.php -lis kept as a fast first pass.editor-appandadmin-app(previously onlybuildran — no quality gate at all).@v6/@v8/@v3/@v2tags).Real fix included
PHPCS surfaced one genuine error: a mispositioned
phpcs:ignoreinclass-rebemer-rest.phpthat did not cover the$placeholdersline of anIN (...)query. The query is safe (literal%splaceholders bound via$wpdb->prepare()); the comment was replaced with a correctly-scopeddisable/enableblock. No runtime behavior change.Testing (run locally: Node 22, PHP 8.4, Composer)
composer phpcs→ 0 errors (2 advisory cache warnings, non-blocking).composer phpstan→ 0 errors with baseline.svelte-check→ 0 errors in both apps.npm audit --audit-level=high→ 0 vulnerabilities (root + both apps).npm test,npm run verify,lint:css,lint:php,check:cheatsheet→ all pass.Notes
composer.json+composer.lockare tracked;vendor/is gitignored.composer phpstan -- --generate-baseline phpstan-baseline.neon.Summary by CodeRabbit