From 1d246603038b682a30b2e60ce1c086b2f60042ef Mon Sep 17 00:00:00 2001 From: Malin Date: Sat, 15 Aug 2026 21:26:46 +0200 Subject: [PATCH] skills(wordpress-plugin-conventions): merge security/lifecycle checks from wordpress/agent-skills Per the 2026-08-15 skills.sh and autoskills.sh scans, both flagged wordpress/agent-skills' wp-plugin-development module (Automattic-origin, now WordPress-org-hosted) as high-value source material for iWP's plugin skill: nonce+capability dual-check discipline, late escaping, prepared SQL, cron idempotency, and uninstall-vs-deactivation guardrails. Adapted (not copied) against real iWP plugin code in wp-plugins/: - nonce+capability must-both framing, cited against class-iwp-cache-db-cleanup.php's actual AJAX handler - late-escaping and wp_unslash()/explicit-key superglobal reading - %i identifier-placeholder version gate (WP 6.2+, most iWP plugins floor at 6.0 or lower) - new "Admin settings" section documenting the real Settings-API vs. AJAX-dashboard split across the suite, since the source's generic Settings-API-first prescription doesn't match roughly half of iWP's plugins - new cron idempotency section citing the existing wp_next_scheduled() guard already used consistently in iwp-cache/iwp-woosales/iwp-booking - new uninstall-vs-deactivation section flagging that only 3 of ~15 plugins ship uninstall.php despite most creating options/tables - new release-packaging checklist tied to iWP's actual IWP_Updater version-wiring convention (header/constant/updater param must agree) Provenance noted inline with source URL. Left out the source's generic architecture/Settings-API prescription and its detect_plugins.mjs script (skill's house style is prose-only, no bundled scripts). Co-Authored-By: Claude Sonnet 5 --- skills/proxmox-admin/SKILL.md | 213 +++++++++++ .../references/command-reference.md | 356 ++++++++++++++++++ skills/wordpress-plugin-conventions/SKILL.md | 157 +++++++- 3 files changed, 724 insertions(+), 2 deletions(-) create mode 100644 skills/proxmox-admin/SKILL.md create mode 100644 skills/proxmox-admin/references/command-reference.md diff --git a/skills/proxmox-admin/SKILL.md b/skills/proxmox-admin/SKILL.md new file mode 100644 index 0000000..962f5ad --- /dev/null +++ b/skills/proxmox-admin/SKILL.md @@ -0,0 +1,213 @@ +--- +name: proxmox-admin +description: Use when administering a Proxmox VE host or cluster via CLI (qm, pct, pvesm, pvecm, pveam, pvesh) — inspecting, creating, or mutating VMs, containers, storage, networking, or cluster state. Safety-first: read-only discovery and explicit target validation before any mutating command, a destructive-command tier list, and real gotchas from hands-on fleet use (reset-vs-reboot tradeoff, Tailscale hostname collisions, storage free-space checks). +license: MIT +source: adapted from https://github.com/bastos/skills/tree/main/proxmox-admin (MIT) — command taxonomy and CLI coverage — plus this fleet's own direct hands-on Proxmox operations across two hosts, which supplied the safety framing, the real gotchas below, and a correction to the source's quick-reference style (see "Why this skill differs from the source" at the bottom) +--- + +# Proxmox VE Administration + +Proxmox VE is a server virtualization platform built on Debian, managing +KVM virtual machines (`qm`) and LXC containers (`pct`) plus storage +(`pvesm`), clustering (`pvecm`), templates (`pveam`), and the API shell +(`pvesh`). This skill covers CLI-based administration. + +**Not for:** Web UI-only workflows (use the CLI equivalents below and in +`references/command-reference.md`). + +**Fleet-specific values (actual VMIDs, storage pool names, bridge/VLAN +IDs, node names) live in this fleet's own `docs/server-*.md` files, not +here.** Every command below uses a placeholder (``, ``, +``, ``) — resolve the real value from live discovery +commands or fleet docs before running anything, never by guessing or +reusing an example number from this file or from upstream documentation. + +## SAFETY FIRST — the order every task follows + +1. **Read-only discovery first, always.** Before touching a single VM or + container, know what actually exists and what state it's in. +2. **Validate the target explicitly.** Never assume a VMID, node name, or + storage name is correct — confirm it via `qm config`/`pct config`/ + `qm list` immediately before acting on it, every time, even if you + "already know" the ID from earlier in the conversation. +3. **Know which tier a command is in before running it.** Destructive-tier + commands (see below) should never run unattended — expect to be + blocked by this environment's own permission classifier, and treat + that as correct behavior to work *with*, not around. + +## Read-only discovery commands (run these before anything else) + +```bash +qm list # all VMs on this node: ID, name, status +qm config # full config of one VM — confirm identity before acting +qm status # current run state + +pct list # all containers on this node +pct config # full config of one container +pct status + +pvesm status # storage pools: type, active, TOTAL/USED/AVAIL +pvecm status # cluster quorum/membership (no-op, harmless, on single-node too) +pvecm nodes # cluster node list + +pvesh get /cluster/resources # everything, via the API shell — useful for cross-node views +``` + +Run the relevant subset of these before every task in this skill, not +just once at the start of a session — state changes, and a VMID that was +stopped five minutes ago may be running now. + +## Never trust a VMID or hostname without confirming it first + +`qm list` gives you IDs and names; `qm config ` gives you the full +picture (disks, storage backing, network, memory). **Always run `qm +config ` (or `pct config `) immediately before any mutating +command against that ID**, even if the ID was already established earlier +in the same task — configs can change between when you last checked and +when you act, and a copy-pasted or misremembered ID silently targeting +the wrong guest is a much worse failure mode than a few extra seconds of +discovery. + +## The Tailscale hostname-collision gotcha + +**Confirmed real risk, hit twice in this session** across the `vulture` +and `ginger` hosts: a bare Tailscale MagicDNS hostname (e.g. `ssh +`) can resolve to the **wrong physical host** if there's a +stale or duplicate device name registered in the tailnet. This is +dangerous specifically because it fails silently — the SSH connection +succeeds, a shell prompt comes back, and everything looks normal; you +just end up running Proxmox commands against a different host's guests +than the one you meant. + +**Before running any command against what you believe is a specific +host** (and especially before anything in the destructive tier below), +verify the connection actually went where you think: + +```bash +ssh -v 2>&1 | grep "Connecting to" +``` + +If a hostname's identity is ever in doubt, skip MagicDNS entirely and +connect by the host's direct Tailscale IP instead — an IP can't collide +the way a device name can. + +## Snapshot vs. backup — know which one you need + +These are not interchangeable, and confusing them is a common source of +false confidence before a destructive operation: + +- **Snapshot** (`qm snapshot`, `qm rollback`, `qm listsnapshot`, + `qm delsnapshot`) — fast, live, stored *on the same storage backend* as + the disk it snapshots. Good for "undo the last five minutes of change" + during an upgrade or config edit. **Does not protect you if the + underlying storage itself fails, fills up, or gets deleted** — the + snapshot dies with it. +- **Backup** (`vzdump`, restored via `qmrestore`/`pct restore`) — a full + archive, ideally written to storage genuinely separate from the guest's + own disk. Slower, but survives storage-level failure and is what you + actually want before anything in the destructive tier below. + +Rule of thumb: before any destructive-tier command, you want a **real +backup on separate storage**, not just a same-storage snapshot. A +snapshot is a convenience for reversible experiments, not a safety net +for irreversible ones. + +## Reset vs. reboot — the real tradeoff learned this session + +- `qm reboot ` sends a graceful ACPI shutdown-then-restart request + *into the guest*. If the guest is hung, has no ACPI daemon running, or + is otherwise unresponsive, **this can hang or time out waiting + indefinitely** — it depends entirely on the guest cooperating. +- `qm reset ` is a hard reset — functionally identical to yanking + power. It was used exactly once this session, deliberately, as the real + fallback when `reboot` wasn't a safe option — but **only after first + confirming the filesystems inside the guest were already cleanly + unmounted**. A hard reset against a guest with dirty/mounted + filesystems risks the same corruption a real power loss would. + +**Do not reach for `reset` just because `reboot` seems slow.** Check what +the guest is actually doing first — console (`qm monitor `), guest +agent (`qm agent ping`), or a direct login — and only escalate to +a hard reset once you've confirmed a clean-unmount state (or have +independently decided the guest's disk state doesn't matter, e.g. it's +about to be destroyed anyway). + +## Storage checks before any disk-affecting operation + +`pvesm status` reports both which storage pools exist **and** their +free space (`TOTAL`/`USED`/`AVAIL` columns) — check both, not just +"does the storage exist." This matters concretely for restores: this +fleet's real VM disks run 200GB+, and a large `qmrestore` can fail +partway through — or silently fill the target storage and start +affecting other guests sharing it — if free space wasn't checked first. +Confirm `pvesm status` shows enough headroom on the *target* storage +before starting any restore, clone, or disk-resize operation, not just +before the operation you're focused on. + +## Destructive-command tier list — expect to be blocked, and that's correct + +**This environment's own permission classifier already blocks +`qmrestore` and other VM-creating/mutating commands, requiring explicit +human confirmation before they run.** This is real, enforced behavior in +this session's environment, not a hypothetical policy. Treat it as a +guardrail to design around, not a friction to route past — if a +destructive-tier command gets blocked, **stop, explain to the human +exactly what the command would do and why it's in this tier, and let +them decide.** Do not look for an equivalent path that avoids the +prompt (a raw `pvesh` API call doing the same mutation, hand-editing +`/etc/pve/qemu-server/.conf` directly, chaining lower-privilege +commands to reach the same end state) — that defeats the guardrail's +purpose rather than satisfying it. + +**Never run these without explicit human confirmation first:** + +- Any `*destroy*`: `qm destroy`, `pct destroy` (with or without + `--purge`) — deletes the guest and its disks, generally unrecoverable + without a separate backup +- Any restore that creates or overwrites a guest: `qmrestore`, + `pct restore` +- Any storage mutation: `pvesm add`, `pvesm remove` +- Any cluster mutation: `pvecm create`, `pvecm add`, `pvecm delnode`, + `pvecm expected` (force-quorum — a single-node recovery action with + real risk of split-brain if used carelessly on a healthy cluster) +- Any migration: `qm migrate`, `pct migrate` — moves a guest between + nodes, can fail mid-flight, and affects a second node's state +- `qm template ` — converting a VM to a template is irreversible +- `qm reset ` — lower severity than destroy, but still surface it + given the hang-vs-corruption tradeoff above; confirm clean-unmount + status even where the environment doesn't hard-block it +- Any bulk/loop pattern (batch-create N containers, batch-destroy, + scripted mass reconfiguration) — higher risk than the single-target + equivalent because the blast radius multiplies silently and a mid-loop + failure can leave a mix of applied and unapplied state + +**Generally fine to run directly** (read-only or narrowly-scoped, +reversible): `qm list`/`config`/`status`, `pct list`/`config`/`status`, +`pvesm status`, `pvecm status`/`nodes`, `pvesh get ...`, `qm agent +ping`/`get-osinfo`, `qm start`/`shutdown`/`stop` on a guest whose +identity you've just confirmed, and snapshot creation (still confirm the +target first — a snapshot against the wrong VMID is a wasted, confusing +action even if not a destructive one). + +## Full command reference + +See `references/command-reference.md` for the complete `qm`/`pct`/ +`pvesm`/`pvecm`/`pveam`/backup/troubleshooting command tables, adapted +from the upstream source with all example VMIDs, storage names, and +bridge names replaced by placeholders. + +## Why this skill differs from the source + +The upstream `bastos/skills@proxmox-admin` skill this was adapted from is +a solid 427-line command taxonomy, but it presents everything — +including `qm destroy`, `pct destroy`, storage mutation, and cluster +operations — in flat quick-reference style with no confirmation, target- +validation, or backup guardrails, and its examples hardcode specific +VMIDs/storage/bridge names as if they were universal defaults to copy. +This version keeps the command coverage but restructures around what +this session's actual hands-on Proxmox work across two hosts (`vulture`, +`ginger`) surfaced as the real operating discipline: discovery before +mutation, explicit target confirmation, a real destructive-tier list that +matches this environment's own enforced permission classifier, and three +gotchas (Tailscale hostname collisions, reset-vs-reboot, storage +free-space checks) that cost real time or carried real risk this session. diff --git a/skills/proxmox-admin/references/command-reference.md b/skills/proxmox-admin/references/command-reference.md new file mode 100644 index 0000000..ea23994 --- /dev/null +++ b/skills/proxmox-admin/references/command-reference.md @@ -0,0 +1,356 @@ +# Proxmox CLI command reference + +Full command coverage for `qm`, `pct`, `pvesm`, `pvecm`, `pveam`, backup/ +restore, networking, firewall, and troubleshooting. Adapted from +`bastos/skills@proxmox-admin` (upstream source: see `SKILL.md` +provenance note) with all example VMIDs, storage pool names, bridge +names, and networks replaced by placeholders — **none of the values +below are this fleet's real values.** Resolve real values from +`docs/server-*.md` or from live discovery (`qm list`, `pvesm status`, +etc.) before running anything. + +Read `SKILL.md` first — it has the safety workflow and the destructive- +command tier list this reference assumes you already know. Everything +in a "never run without confirmation" tier there is repeated here only +as reference syntax, not as something to execute unattended. + +## Tool overview + +| Tool | Purpose | +|------|---------| +| `qm` | Manage KVM virtual machines | +| `pct` | Manage LXC containers | +| `pvesm` | Manage storage | +| `pvecm` | Manage cluster | +| `pveam` | Manage appliance/template downloads | +| `pvesh` | Access the Proxmox API from the shell | +| `pveperf` | Benchmark host performance | + +## VM management with `qm` + +### Creating a VM (destructive tier: creates persistent state — confirm first) + +```bash +qm create --name --memory 2048 --cores 2 --sockets 1 \ + --net0 virtio,bridge= --ostype l26 + +# SCSI disk on a given storage pool +qm create --name --memory 4096 --cores 4 \ + --scsi0 :32 --scsihw virtio-scsi-pci \ + --net0 virtio,bridge= --ostype l26 + +# Attach an ISO for installation +qm set --cdrom :iso/.iso --boot order=ide2 +``` + +### VM lifecycle + +| Command | Purpose | Tier | +|---------|---------|------| +| `qm start ` | Start a VM | generally fine once target confirmed | +| `qm shutdown ` | Graceful ACPI shutdown | generally fine | +| `qm stop ` | Force stop (like pulling power) | confirm first — no ACPI grace | +| `qm reboot ` | Reboot (ACPI-graceful) | can hang if guest unresponsive — see SKILL.md | +| `qm reset ` | Hard reset | confirm clean unmount first — see SKILL.md | +| `qm suspend ` | Suspend to RAM | confirm first | +| `qm resume ` | Resume from suspend | generally fine | +| `qm destroy ` | Delete VM and its disks | **never without human confirmation** | +| `qm destroy --purge` | Delete VM, disks, and all related jobs | **never without human confirmation** | + +### VM configuration + +```bash +qm config # show current config — always run before qm set + +qm set --memory 8192 +qm set --cores 4 +qm set --balloon 2048 # dynamic memory (min) +qm set --cpu cputype=host # pass through host CPU features +qm set --machine q35 # Q35 chipset (needed for PCIe passthrough) + +# Add/resize disks — check pvesm status for free space first +qm set --scsi1 :50 # add a 50GB disk +qm disk resize scsi0 +20G # grow an existing disk + +# Networking — and any VLAN tag are fleet-specific, resolve first +qm set --net0 virtio,bridge=,tag= +qm set --net1 virtio,bridge= + +# Cloud-init +qm set --ide2 :cloudinit +qm set --ciuser --cipassword '' +qm set --ipconfig0 ip=/,gw= +qm set --sshkeys ~/.ssh/authorized_keys +qm set --boot order=scsi0 + +# EFI / UEFI boot +qm set --bios ovmf --efidisk0 :1,efitype=4m,pre-enrolled-keys=1 + +# Serial console (headless) +qm set --serial0 socket --vga serial0 + +# PCI passthrough +qm set --hostpci0 ,pcie=1 +``` + +### Snapshots and cloning + +See SKILL.md's "Snapshot vs. backup" section before relying on any of +these as your only safety net. + +```bash +qm snapshot --description "" +qm listsnapshot +qm rollback +qm delsnapshot + +qm clone --name --full # full copy +qm clone --name # linked clone (shares base disk) +``` + +### Templates + +```bash +qm template # convert VM to template — IRREVERSIBLE, confirm first + +qm clone --name # linked clone from template +qm clone --name --full # full clone from template +``` + +### Migration (confirm first — affects a second node) + +```bash +qm migrate --online # online (live) migration +qm migrate # offline migration +``` + +### Monitoring + +```bash +qm status +qm list + +qm agent ping +qm agent get-osinfo + +qm monitor # QEMU monitor console +``` + +## Container management with `pct` + +### Creating a container (destructive tier — confirm first) + +```bash +# Download a template first +pveam update +pveam available --section system +pveam download + +pct create :vztmpl/ \ + --hostname --memory 1024 --cores 2 \ + --rootfs :8 \ + --net0 name=eth0,bridge=,ip=dhcp \ + --password '' --unprivileged 1 + +# Static IP variant +pct create :vztmpl/ \ + --hostname --memory 2048 --cores 2 \ + --rootfs :16 \ + --net0 name=eth0,bridge=,ip=/,gw= \ + --nameserver --unprivileged 1 +``` + +### Container lifecycle + +| Command | Purpose | Tier | +|---------|---------|------| +| `pct start ` | Start container | generally fine once target confirmed | +| `pct shutdown ` | Graceful shutdown | generally fine | +| `pct stop ` | Force stop | confirm first | +| `pct reboot ` | Reboot container | same ACPI-hang caveat as `qm reboot` | +| `pct destroy ` | Delete container and its volumes | **never without human confirmation** | +| `pct enter ` | Open a shell inside the container | generally fine | +| `pct exec -- ` | Run a command inside the container | depends what `` does | +| `pct console ` | Attach to container console | generally fine | + +### Container configuration + +```bash +pct config # always run before pct set + +pct set --memory 4096 +pct set --cores 4 +pct set --swap 1024 + +pct set --mp0 /mnt/data,mp=/data # bind mount from host +pct set --mp1 :50,mp=/var/lib/data # additional storage volume + +pct set --net0 name=eth0,bridge=,ip=/,gw= +pct set --net1 name=eth1,bridge=,ip=dhcp + +pct set --features nesting=1 +pct set --features nesting=1,fuse=1,mount=nfs + +pct set --nameserver " " --searchdomain + +pct set --onboot 1 --startup order=1,up=30 +``` + +### Container snapshots and cloning + +```bash +pct snapshot +pct rollback +pct clone --hostname --full +``` + +## Storage management + +```bash +pvesm status # list pools + free space — check before any restore/resize + +pvesm list # content of a specific storage pool + +# Adding storage is a cluster-wide config mutation — confirm first +pvesm add dir --path --content backup +pvesm add nfs --server --export --content images,vztmpl +pvesm add lvm --vgname --content rootdir,images +pvesm add zfspool --pool / --content rootdir,images + +pvesm remove # never without human confirmation + +wget -P /var/lib/vz/template/iso/ +``` + +## Networking + +```bash +cat /etc/network/interfaces # current bridge/interface config — read first + +# Bridge stanza shape (values are placeholders, not this fleet's real config) +# auto +# iface inet static +# address / +# bridge-ports +# bridge-stp off +# bridge-fd 0 + +ifreload -a # apply network changes — can drop connectivity if misconfigured, confirm first +``` + +## Cluster management (all tiers below: confirm first — affects multiple nodes) + +```bash +pvecm create +pvecm add +pvecm status +pvecm nodes +pvecm delnode +pvecm expected 1 # force quorum — single-node recovery only, real split-brain risk otherwise +``` + +## Firewall + +```bash +pve-firewall start +pve-firewall stop +pve-firewall status + +# Datacenter: /etc/pve/firewall/cluster.fw ([OPTIONS] enable: 1) +# Node: /etc/pve/nodes//host.fw +# VM/CT: /etc/pve/firewall/.fw +# +# Add remote-management allow rules BEFORE enabling a restrictive default +# policy — locking yourself out of remote access to the node is a real, +# hard-to-recover-from failure mode. +``` + +## Backup and restore + +```bash +vzdump --storage --mode snapshot --compress zstd +vzdump --storage --mode stop --compress zstd +vzdump --all --storage --mode snapshot --compress zstd --mailto + +# Restore — never without human confirmation (creates/overwrites a guest). +# Check pvesm status for target free space FIRST, especially for large disks. +qmrestore --storage +pct restore +``` + +## Common provisioning patterns (reference only — each step still follows the tier rules above) + +### Cloud-init VM from a template + +```bash +qm template # irreversible — confirm first + +qm clone --name --full +qm set --ciuser --sshkeys ~/.ssh/authorized_keys +qm set --ipconfig0 ip=/,gw= +qm set --nameserver +qm start +``` + +Note: some Proxmox documentation and community examples use VMIDs like +`9000` as a convention for template base images. That's a naming +convention some operators adopt, not a Proxmox default or a value to +copy into this fleet — check `docs/server-*.md` for whatever convention +(if any) applies to a given host before picking an ID. + +### Batch-create containers + +Bulk/loop patterns are explicitly a higher tier than the single-target +equivalent (see SKILL.md) — pilot one iteration manually, confirm it's +correct, before running the loop. + +```bash +for i in $(seq 1 ); do + CTID=$(( + i)) + pct create "$CTID" :vztmpl/ \ + --hostname "-${i}" --memory 1024 --cores 2 \ + --rootfs :8 \ + --net0 name=eth0,bridge=,ip=.$(( + i))/,gw= \ + --unprivileged 1 --start 1 +done +``` + +### Import a disk image (e.g. a cloud image) + +```bash +wget + +qm disk import +qm set --scsi0 :vm--disk-0 +qm set --boot order=scsi0 +``` + +## Troubleshooting + +| Problem | Solution | +|---------|----------| +| VM won't start | `qm config `, verify storage exists with `pvesm status` | +| "TASK ERROR: can't lock file" | Check for a genuinely running task first; if the task is gone, `qm unlock ` / `pct unlock ` | +| Container has no network | Check bridge exists: `brctl show`; verify firewall rules | +| Disk full on storage | `pvesm status` for usage; `lvs` for LVM thin pools | +| Cluster quorum lost | `pvecm expected 1` on a surviving node — single-node recovery only, confirm first | +| Migration fails | Check CPU type compatibility; confirm network reachability between nodes | +| Backup fails with lock error | `qm unlock ` / `pct unlock ` | +| Slow disk I/O in VM | `--scsihw virtio-scsi-single` plus disk options like `,iothread=1,discard=on` | +| Guest agent not responding | Install `qemu-guest-agent` in the guest, then `qm set --agent 1` | +| SSH/command seems to hit the wrong host | See SKILL.md's Tailscale hostname-collision gotcha — verify with `ssh -v 2>&1 \| grep "Connecting to"` | + +## Useful paths + +| Path | Contents | +|------|----------| +| `/etc/pve/` | Cluster-wide config (pmxcfs) | +| `/etc/pve/qemu-server/.conf` | VM configuration files | +| `/etc/pve/lxc/.conf` | Container configuration files | +| `/etc/pve/storage.cfg` | Storage definitions | +| `/etc/pve/nodes/` | Per-node configuration | +| `/var/lib/vz/` | Default local storage root | +| `/var/lib/vz/template/iso/` | ISO images | +| `/var/lib/vz/template/cache/` | Container templates | +| `/var/lib/vz/dump/` | Backup files | +| `/var/log/pve/tasks/` | Task logs | diff --git a/skills/wordpress-plugin-conventions/SKILL.md b/skills/wordpress-plugin-conventions/SKILL.md index 599f082..c51b494 100644 --- a/skills/wordpress-plugin-conventions/SKILL.md +++ b/skills/wordpress-plugin-conventions/SKILL.md @@ -81,15 +81,57 @@ find/replace, etc.) before they ever reach a live site. `current_user_can('manage_options')` (or the narrowest capability that actually applies — don't default to `manage_options` for things a lower-privileged role should legitimately be able to do). +- **Always both, never either alone.** A nonce proves the request came + from your own form/page (CSRF protection) — it says nothing about who + is allowed to make it. A capability check proves authorization — it + says nothing about whether the request was forged. Nonce-only lets a + forged/leaked-nonce request through if the attacker can get one + in-scope; capability-only lets a logged-in admin's browser be tricked + into firing the action via CSRF. This is already the live pattern — + `class-iwp-cache-db-cleanup.php`'s AJAX handler runs + `check_ajax_referer('iwp_cache_db_cleanup', 'nonce')` immediately + followed by `current_user_can('manage_options')`, never one without + the other. Match that shape, don't drop either check because "the + button is already hidden from non-admins" — client-side hiding is not + a server-side check. - Escape on output, every time, using the context-correct function: `esc_html()`, `esc_attr()`, `esc_url()`, `esc_js()` — never raw-echo anything that traces back to user input or the database without one of - these. + these. Escape **late** — at the point of output, not when the value is + first read or stored — so the stored/cached copy stays raw and every + new rendering context gets its own correct escaping function. - Sanitize on input: `sanitize_text_field()`, `absint()`, `sanitize_email()`, etc. — appropriate to the expected shape of the data, applied at the point the `$_POST`/`$_GET` value is first read. + Read superglobals by explicit key only — never loop over or dump the + whole `$_POST`/`$_GET` array — and run `wp_unslash()` before + sanitizing (WordPress adds slashes to superglobal values; sanitizing + before unslashing leaves stray backslashes in what gets stored). - `$wpdb->prepare()` for every query with a variable in it — no string- - interpolated SQL, ever, no exceptions. + interpolated SQL, ever, no exceptions. The `%i` identifier placeholder + (for table/column names) only exists from WP 6.2 — most iWP plugins + declare `Requires at least: 6.0` or no floor at all, so don't rely on + `%i` without either raising the plugin's stated minimum or falling + back to an allow-listed identifier switch instead. + +## Admin settings: Settings API vs. AJAX-backed dashboards + +iWP plugins split roughly evenly between two real patterns — check which +one a given plugin already uses before adding a settings field, don't +introduce a third: + +- **Classic Settings API** (`iwp-mailer`, `iwp-mailer-relay`, + `iwp-woosales`, `iwp-subscriptions`): `register_setting()` with a + `sanitize_callback`, an admin-post form, capability enforced by the + standard `option_page_capability_*` gate. +- **AJAX-backed dashboard** (`iwp-cache`, `iwp-security`, and most of + the newer plugins): a JS dashboard posts to `wp_ajax_*` handlers that + manually replicate the same discipline — `check_ajax_referer()` + + `current_user_can()` at the top of every handler, then + `update_option()` directly. `class-iwp-cache-db-cleanup.php` is the + canonical shape. + +Either is fine; match whichever the plugin you're touching already uses. ## Database tables @@ -103,6 +145,66 @@ dbDelta($sql); Track a `DB_VERSION` constant + a stored option so future plugin updates can detect and re-run `dbDelta()` for schema changes. +## Cron: every scheduled hook needs a guard, and the callback must survive running twice + +Every iWP plugin that schedules a recurring event already follows the +same guard — wrap `wp_schedule_event()` in a `wp_next_scheduled()` check, +never schedule unconditionally on every request: +```php +if (!wp_next_scheduled(self::CRON_HOOK)) { + wp_schedule_event(time(), 'daily', self::CRON_HOOK); +} +``` +(see `iwp-cache/includes/class-iwp-cache-preload.php`, +`iwp-woosales/includes/class-iwp-woosales-cron.php`, +`iwp-booking/includes/class-iwp-booking-cron.php`). Without the guard, +every page load or re-activation re-schedules the event, and WP-Cron +silently accumulates duplicate instances of the same hook — they all +fire, so a "daily" job can end up running several times a day. + +Beyond the schedule guard, the callback itself must tolerate running +late or twice — WP-Cron is a request-triggered pseudo-cron, not a +real-time scheduler; a low-traffic site can miss its window for hours, +and two concurrent requests can trigger the same due hook back-to-back. +Make the job naturally idempotent (e.g. "sync everything modified since +`last_sync_time`", safe to re-run) or guard the actual work with a +short-lived transient/option lock, not just the scheduling call. + +Always pair `wp_schedule_event()` with the matching +`wp_clear_scheduled_hook()` — from `register_deactivation_hook()` if the +schedule shouldn't survive deactivation, or from `uninstall.php` if it +should survive deactivation but not a full uninstall (see next section). + +## Uninstall vs. deactivation: decide data retention explicitly + +Only three plugins in this suite currently ship an `uninstall.php` +(`iwp-security`, `iwp-woosales`, `informatiq-toolkit`), even though most +plugins create options, transients, and in several cases their own +database tables. That split isn't automatically wrong — WordPress's own +convention is that **deactivation should be reversible and +non-destructive** (stop cron, leave data alone so re-activating restores +prior state) while **uninstall is where actual cleanup belongs** — but +it should be a decision made per plugin, not an oversight. + +When adding a new plugin, or touching an existing one's lifecycle code, +decide and make explicit which applies: + +- **Deactivation** (`register_deactivation_hook`): stop cron + (`wp_clear_scheduled_hook()`) and nothing else. Data stays so toggling + the plugin off temporarily doesn't lose settings/history. +- **Uninstall** (`uninstall.php`, gated on + `if (!defined('WP_UNINSTALL_PLUGIN')) exit;` — prefer this over + `register_uninstall_hook()` for anything nontrivial, since a flat file + is simpler to keep in sync with the plugin's actual option/table list): + delete every option and transient the plugin created, drop every + custom table (`DROP TABLE IF EXISTS`), clear any cron still scheduled. + See `iwp-security/uninstall.php` and `iwp-woosales/uninstall.php` for + the shape. + +A plugin that creates a custom table or a nontrivial option set and ships +neither an `uninstall.php` nor an explicit "intentionally retained, +because X" note is an oversight worth flagging, not a policy. + ## PHP 7.4 compatibility Unless a specific brief says otherwise, target PHP 7.4+ (a large fraction @@ -187,3 +289,54 @@ WooCommerce context), object caching (`wp_cache_*` functions), HTTP requests (`wp_remote_get`/`wp_remote_post`, not raw `curl`) — check whether a core API already does it. Reinventing these is both wasted effort and a common source of subtle bugs core already solved correctly. + +## Release packaging: version consistency before you zip anything + +iWP plugins auto-update through `IWP_Updater` +(`wp-plugins/iwp-updater-client`, bundled per-plugin as +`includes/class-iwp-updater.php`), which reads the plugin's version from +a PHP constant, not from the zip filename or a release note. A release +is only as good as that constant being right, so before packaging: + +1. **Three places must agree**: the `Version:` plugin header, the + `PREFIX_VERSION` constant defined right below the `ABSPATH` guard, and + the `'version' => PREFIX_VERSION` line passed into + `new IWP_Updater([...])`. Every plugin in the suite wires the + updater's version to that same constant, never a separate literal — + e.g. `iwp-cache.php` passes `'version' => IWP_CACHE_VERSION`, + `iwp-booking.php` passes `'version' => IWP_BOOKING_VERSION`. Bump the + header without bumping the constant (or vice versa) and + `IWP_Updater`'s `pre_set_site_transient_update_plugins` filter + compares the wrong number — customers either get nagged when already + current, or don't get notified of a real update. +2. **`plugin_slug` must match the catalog key** on iwp.es — + `wp-plugins/iwp-subscriptions`'s update-check endpoint looks the slug + up directly. Confirm against the actual product catalog, don't assume + it matches the plugin's directory name. +3. **Exclude dev artifacts from the shipped zip**: `.git/`, `.gitignore` + itself, `node_modules/`, test fixtures, and any internal-only + `README.md` content not meant for a customer (some plugin READMEs in + this suite are customer-facing, some — like + `iwp-updater-client/README.md` — are internal integration notes). +4. **Verify the packaged zip before shipping it**, don't trust a clean + build as proof it activates cleanly — deploy it with + `wordpress-plugin-staging-verification` against the persistent + `staging1` baseline. If this release is a rebrand/fork rather than a + version bump on an existing iWP plugin, run + `wordpress-plugin-rebrand`'s identifier-renaming/flattening step + first. + +## Provenance + +The nonce+capability pairing framing, late-escaping wording, `%i` +placeholder caveat, cron-idempotency guardrail, and uninstall-vs- +deactivation framing above were adapted from +[wordpress/agent-skills](https://github.com/wordpress/agent-skills)' +`wp-plugin-development` skill +(https://skills.sh/wordpress/agent-skills/wp-plugin-development, surfaced +by the 2026-08-15 skills.sh/autoskills.sh scans) — reworked against this +suite's actual code rather than copied verbatim. Its generic +"Settings-API-first" architecture prescription was deliberately not +imported wholesale: roughly half of iWP's plugins use an AJAX-backed +dashboard pattern instead (see "Admin settings" above), and prescribing +one true pattern would contradict the real, working code.